[go_router] Cleans up route match API and introduces dart fix

[chunhtai]

Clean up API around RouteMatch/RouteMatchList/GoRouterState,

This is a breaking change that renamed some of the GoRouterState property to have a more descriptive name as flutter style guide suggested https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#avoid-abbreviations

also introducing dart fix to help with migration

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
• I signed the CLA.
• The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• I listed at least one issue that this PR fixes in the description above.
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
• I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this PR is test-exempt.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[johnpryan]

This is causing issues with CI: "Unexpected local analysis options"

[chunhtai]

@stuartmorgan can you take a look at the ci setup?

[chunhtai]

I am not sure if this is the right place to disable the lint

[stuartmorgan]

I think you could make a custom analysis_options.yaml in packages/go_router/ that includes this one, and then adds this exclude. Then you'd need to add go_router to script/configs/custom_analysis.yaml.

That would make it more obvious locally that something strange is going on with analysis.

[chunhtai]

hmm, this doesn't work. In order to let dart fix --compare-to-golden to work. I can't ignore the lint using analysis_options.yaml as the dart fix would only touch the code that has analyzer error or warning. That means I can't do any kind or ignore that would affect dart analyze.

I took a look at the flutter repro, it looks like they wrote the custom analyzer script to ignore these lint.

I think I will have to need to update the code in flutter_plugin_tools to ignore the folder when running analyze.

Do you have any other idea that I should do?

[chunhtai]

nvm, it seems like i can use some analyzer-options trick to get around this

[stuartmorgan]

I think you could make a custom analysis_options.yaml in packages/go_router/ that includes this one, and then adds this exclude. Then you'd need to add go_router to script/configs/custom_analysis.yaml.

That would make it more obvious locally that something strange is going on with analysis.

[stuartmorgan]

If you want custom, package-specific tests, they should be in your package, not in the CI config. The mechanism that's in place for that is the custom-test repo tooling command, which looks for a tool/run_tests.dart in your package, and runs it if it's there. packages/cross_file/tool/run_tests.dart is a simple example you can follow. (There's a legacy mode that uses a run_script.sh, but please do not add a new shell-based script. Those are only there because they predate standardizing on repo tooling.)

By default it will run on every host platform, so if you only run it on Linux you should check for Platform.isLinux in your script, and no-op if it's not Linux.

[chunhtai]

Hi @johnpryan , I updated some code, can you take another look

Hi @stuartmorgan , can you take a look at the ci setup again, I addressed all comments

[chunhtai]

would update to flutter go link once website is unfrozen

[stuartmorgan]

The CI parts LGTM with small comments.

[stuartmorgan]

Optional: I would prefer the platform check to be in the script (e.g., exit immediately if (!Platform.isLinux)), with an explanation there as to why it's platform-limited, to make the logic as localized as possible. This exclude system was needed because we still have some .sh scripts, but I hope to eventually eliminate the custom test exclude files.

[stuartmorgan]

This is not accurate.

[stuartmorgan]

Nit: **BREAKING CHANGES**:

[stuartmorgan]

Nit: missing .

[auto-submit]

auto label is removed for flutter/packages, pr: 3819, due to - The status or check suite repo_checks has failed. Please fix the issues identified (or deflake) before re-applying this label.

[ahmednfwela]

@chunhtai I just tried v7.0.0 and dart fix isn't getting computed at all

>  dart fix --apply   
Computing fixes in example...
Nothing to fix!

[chunhtai]

@ahmednfwela thanks for reporting. It is weird that It does work if I point the go_router to local directory in pubspec.yaml. I filed a bug dart-lang/sdk#52233

For now, i think you can workaround it by doing a local check out of the go_router and override your dependencies in pubspec.yaml with the path, and run flutter pub get and dart fix --apply again

[ahmednfwela]

@chunhtai nope, doesn't even work when importing path

[chunhtai]

@ahmednfwela

Can you try git clone the entire package https://github.com/flutter/packages and then link the dependency into the go_router inside the local copy of the repo?

you will need to do flutter pub get to make sure the go_router reference is properly updated

[ahmednfwela]

1. I cloned the entire package repo to E:\Projects\Packages\flutter_packages
2. went to packages/go_router/example/lib/named_routes.dart in the cloned repo
3. renamed state.pathParameters to state.params
4. Lint appears in vscode
   

---

1. In my project
2. add dependency override:

dependency_overrides: 
  go_router:
    path: E:\Projects\Packages\flutter_packages\packages\go_router

3. flutter pub upgrade
   correct path appears:

Warning: You are using these overridden dependencies:
! go_router 7.0.0 from path E:\Projects\Packages\flutter_packages\packages\go_router

4. go to a similar state.queryParams entry
5. lints don't appear
   

[chunhtai]

@ahmednfwela I didn't use dependency_override, can you try using the path in
dependencies: in pubspec yaml directly?

[ahmednfwela]

@chunhtai doesn't make a difference

[chunhtai]

I am sorry to hear that. since this is likely a dart sdk issue, there isn't much I can do to fix this. I will update the migration doc with more detail steps.

[ahmednfwela]

btw here is my environment, so maybe it's a windows only problem ?

flutter doctor -v

[√] Flutter (Channel stable, 3.7.12, on Microsoft Windows [Version 10.0.19045.2846], locale en-US)
    • Flutter version 3.7.12 on channel stable at C:\tools\flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 4d9e56e694 (2 weeks ago), 2023-04-17 21:47:46 -0400
    • Engine revision 1a65d409c7
    • Dart version 2.19.6
    • DevTools version 2.20.1

[√] Windows Version (Installed version of Windows is version 10 or higher)

[√] Android toolchain - develop for Android devices (Android SDK version 31.0.0-rc1)
    • Android SDK at C:\Users\ahmed\AppData\Local\Android\Sdk
    • Platform android-33, build-tools 31.0.0-rc1
    • ANDROID_HOME = C:\Users\ahmed\AppData\Local\Android\Sdk
    • Java binary at: C:\Program Files\Android\Android Studio\jre\bin\java
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)
    • All Android licenses accepted.

[√] Chrome - develop for the web
    • Chrome at C:\Program Files (x86)\Google\Chrome\Application\chrome.exe

[√] Visual Studio - develop for Windows (Visual Studio Community 2022 17.5.4)
    • Visual Studio at C:\Program Files\Microsoft Visual Studio\2022\Community
    • Visual Studio Community 2022 version 17.5.33530.505
    • Windows 10 SDK version 10.0.22000.0

[√] Android Studio (version 2021.3)
    • Android Studio at C:\Program Files\Android\Android Studio
    • Flutter plugin can be installed from:
       https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
       https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)

[√] VS Code, 64-bit edition (version 1.77.3)
    • VS Code at C:\Program Files\Microsoft VS Code
    • Flutter extension version 3.15.0

[√] Connected device (3 available)
    • Windows (desktop) • windows • windows-x64    • Microsoft Windows [Version 10.0.19045.2846]
    • Chrome (web)      • chrome  • web-javascript • Google Chrome 112.0.5615.138
    • Edge (web)        • edge    • web-javascript • Microsoft Edge 112.0.1722.64

[√] HTTP Host Availability
    • All required HTTP hosts are available

• No issues found!