[go_router] Add GoRouterState state parameter to GoRouterData.onExit

[ValentinVignal]

Part of flutter/flutter#137394

I need to add the state parameter to the onExit method so I can use the factoryImpl(state):

packages/packages/go_router/lib/src/route_data.dart

Lines 100 to 118 in d4cd4f0

	Widget builder(BuildContext context, GoRouterState state) =>
	factoryImpl(state).build(context, state);
	Page<void> pageBuilder(BuildContext context, GoRouterState state) =>
	factoryImpl(state).buildPage(context, state);
	FutureOr<String?> redirect(BuildContext context, GoRouterState state) =>
	factoryImpl(state).redirect(context, state);
	return GoRoute(
	path: path,
	name: name,
	builder: builder,
	pageBuilder: pageBuilder,
	redirect: redirect,
	routes: routes,
	parentNavigatorKey: parentNavigatorKey,
	);
	}

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 linked to 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.

[chunhtai]

code LGTM, but we should have a bit more test.

Since this is a breaking change can you write a breaking change doc?

You can also explore dart fix, I think you can add a new rule there to auto migrate customers' code.

[chunhtai]

Can you add a new test that assert the properties in state, to make sure the state is accurate with the route?

[chunhtai]

See https://github.com/flutter/packages/blob/main/packages/go_router/lib/fix_data.yaml

if you can write a rule that cover all cases, the migration doc can be as simple as please run dart fix --apply

[ValentinVignal]

Since this is a breaking change can you write a breaking change doc?

Okay sure I'll write one ! 👍

[ValentinVignal]

@chunhtai I'm struggling a bit with the dart fix. Is there a way to add a parameter to a method given to a field of a class constructor ? I didn't find it in the doc

(I also posted a message on discord https://discord.com/channels/608014603317936148/608021635370582020/1228715527073955912)

[chunhtai]

I think you can probably reuse some of the code from this example https://github.com/flutter/flutter/blob/2e748e8598b3169d5d186b8a4d02e603b62653d0/packages/flutter/lib/fix_data/fix_cupertino.yaml#L182

[chunhtai]

you can also check the syntax here https://github.com/flutter/flutter/wiki/Data-driven-Fixes

[ValentinVignal]

See https://github.com/flutter/packages/blob/main/packages/go_router/lib/fix_data.yaml

if you can write a rule that cover all cases, the migration doc can be as simple as please run dart fix --apply

That's a bit different, it looks like the addParameter is used to change occurrences of:

MyClass.myMethod(param1);

into

MyClass.myMethod(param1, param2);

while I need to add a parameter to a method given as a argument:

GoRouter(
  onExit: (BuildContext context) => true;
);

into

GoRouter(
  onExit: (BuildContext context, GoRouterState state) => true;
);

[ValentinVignal]

@chunhtai I've made a migration guide for the go router v14:

• https://docs.google.com/document/d/1Z6RYo7rGIdtryvQvAntekF53zoz4iy4XvIamBxRWa4Y/edit?usp=sharing&resourcekey=0-CH_yB7ur4gLvSuqPtB5bZA
• Add the go router v14 migration guide website#10423
• Add the go router v14 migration guide flutter#146805

[ValentinVignal]

The migration doc flutter/website#10423 got merged

[chunhtai]

LGTM, thanks for improving the API!

[hangyujin]

LGTM