[go_router] Add `buildPageWithState`

[ValentinVignal]

This should close https://github.com/flutter/flutter/issues/109440

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

• [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
• [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
• [x] 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.)
• [x] I signed the CLA.
• [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• [x] I listed at least one issue that this PR fixes in the description above.
• [x] I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
• [x] I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
• [x] 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.
• [x] All existing and new tests are passing.

[flutter-dashboard[bot]]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[ValentinVignal]

This is a "dummy" implementation because I don't know what you would prefer.

Do you want me to change the existing buildPage method? But it would be a breaking change. Should I create a new method as I did? If yes, do you prefer another name than buildPageWithState ?

For the tests, I guess their implementation/logic would change depending on what you tell me to do. I'll write them once I know what I should implement

[ValentinVignal]

Thank you @chunhtai Should I deprecate buildPage ?

[ValentinVignal]

I deprecated buildPage and the analyzer is failing because of that. What should I do about it? Add // ignore: ... ?

[chunhtai]

I deprecated buildPage and the analyzer is failing because of that. What should I do about it? Add // ignore: ... ?

for the reference in doc string, please update it to use buildPageWithState. For the code that uses it, you can add ignore if it is unavoidable; otherwise, they should use buildPageWithState.

You may need to add ignore to the go_router_builder first

[ValentinVignal]

@chunhtai I replaced the references in the doc strings and ignore the call to builPage in https://github.com/flutter/packages/pull/2444/commits/32b94eb988ab8bdd1d2efb6a0815cf29ab8c57f7

[ValentinVignal]

the go_router_builder may need to be marked //ignore first.

I'm sorry, I don't understand what you mean by that

[ValentinVignal]

the go_router_builder may need to be marked //ignore first.

I'm sorry, I don't understand what you mean by that

@chunhtai is https://github.com/flutter/packages/pull/2444/commits/d5f9c9261eee7860739ae93d44649e3596aba9e3 what you meant ?

[chunhtai]

the go_router_builder may need to be marked //ignore first.

I'm sorry, I don't understand what you mean by that

@chunhtai is d5f9c92 what you meant ?

yes.

[chunhtai]

@ValentinVignal the ignore in go_router_builder should be in a separate pr that needs to go in first.

[ValentinVignal]

@ValentinVignal the ignore in go_router_builder should be in a separate pr that needs to go in first.

Ok, I've create this PR for the ignore in go_router_builder https://github.com/flutter/packages/pull/2504 I'm just pushing the fix for the merge conflicts for now

[ValentinVignal]

@chunhtai https://github.com/flutter/packages/pull/2504 got merged, so I guess this PR can be re-reviewed

[chunhtai]

can you rebase off the main branch to fix the ci.yaml failure?

[ValentinVignal]

@chunhtai Can I do a merge or does it have to be a rebase?

[chunhtai]

rebase will be cleaner and that is what I usually do. If you can manage merge without causing commits hell, then it is up to you.

[ValentinVignal]

@chunhtai I merged main in my branch in https://github.com/flutter/packages/pull/2444/commits/3802759ddfd06eef5842c7ad2fd89d20b8a18a82 And I think I managed to merge without causing commits hell. Tell me if that is good for you or not