Skip to content

fix(router): warn when customRouteBuilder is used with the Navigator 1 router - #78

Open
Eimen2018 wants to merge 4 commits into
mainfrom
fix/1164-custom-route-builder-navigator1
Open

Eimen2018 wants to merge 4 commits into
mainfrom
fix/1164-custom-route-builder-navigator1

Conversation

@Eimen2018

Copy link
Copy Markdown
Collaborator

Summary

Refs Stacked-Org/stacked#1164: CustomRoute.customRouteBuilder is silently ignored by the default (Navigator 1) router generator, which always emits a PageRouteBuilder. Only the Navigator 2 generator honours it.

Why not just support it in Navigator 1

The builder contract is Route<T> Function<T>(BuildContext context, Widget child, CustomRoute page). Navigator 1's onGenerateRoute(RouteSettings) has no BuildContext, so the generated router has nothing to call the builder with. Supporting it there would require a second, incompatible builder signature, which would be confusing and a breaking change for anyone with a builder written for Navigator 2. That was tried first and abandoned.

Change

router_class_builder_helper.dart: generated output is unchanged. When customRouteBuilder is set on a route in the Navigator 1 path, the generator now prints a build-time warning, using the same print('WARNING => ...') convention already used in route_config_resolver.dart:

WARNING => CustomRoute.customRouteBuilder on 'BottomSheetView' is only supported by the Navigator 2 router.
Set `navigator2: true` in your @StackedApp / stacked config to use it; it is ignored by the default router.

No throw, so builds that succeeded before still succeed.

Tests

router_class_builder_helper_test.dart: output with and without customRouteBuilder is byte-identical; the warning is printed when the builder is set (captured via runZoned print interception); no warning when unset.

Verification

  • dart test: 330 passed, 1 pre-existing skip
  • dart analyze lib test: no new issues

Note for reviewers: dev_dependencies: freezed: ^4.0.0 on main requires Dart 3.12, which the current Flutter stable does not ship. I ran the suite with a temporary local downgrade and reverted pubspec.yaml before committing.

🤖 Generated with Claude Code

…1 router

The default router generator silently ignored CustomRoute.customRouteBuilder because Navigator 1's onGenerateRoute has no BuildContext to invoke the builder with. Emit a build-time warning pointing users to navigator2: true instead of dropping the option silently.

Refs Stacked-Org/stacked#1164

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

LCOV of commit 80d7473 during Test Package #227

📊 Summary Coverage

Metric Coverage Details
📈 Lines 🔴 64.7% 1616 / 2496 lines
🔧 Functions ⚪ N/A no data found
🌿 Branches ⚪ N/A no data found

📁 Files Changed Coverage

File Lines Functions Branches
No coverage data for changed files

🟢 ≥ 90% · 🟡 ≥ 70% · 🔴 below 70% · ⚪ no data
Full coverage report

@Eimen2018
Eimen2018 force-pushed the fix/1164-custom-route-builder-navigator1 branch from f4473c9 to bf3ff6e Compare September 18, 2026 21:24
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@Eimen2018
Eimen2018 force-pushed the fix/1164-custom-route-builder-navigator1 branch from bf3ff6e to d7afbef Compare September 18, 2026 21:24
// ignore: avoid_print
print(
'\nWARNING => CustomRoute.customRouteBuilder on \'${route.routeName}\' is only supported by the Navigator 2 router.\n'
'Set `navigator2: true` in your @StackedApp / stacked config to use it; it is ignored by the default router.\n');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warning's remediation is wrong. navigator2 is not a @StackedApp parameter (StackedApp in stacked_shared has no such field). It is a builder option: GeneratorConfig.fromJson(options.config) reads it in lib/builder.dart, so it lives in build.yaml under targets: $default: builders: stacked_generator|stackedRouterGenerator: options: navigator2: true. A user who follows this message and writes @StackedApp(navigator2: true, ...) gets a compile error ("no named parameter 'navigator2'"). "stacked config" (stacked.json) doesn't control it either. Please point to the build.yaml builder option instead.

Suggested change
'Set `navigator2: true` in your @StackedApp / stacked config to use it; it is ignored by the default router.\n');
'Set `navigator2: true` in the stacked_generator|stackedRouterGenerator builder options in build.yaml to use it; it is ignored by the default router.\n');

navigator2 is a stacked_generator builder option, not a @stackedapp parameter.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
final generatedWithout = routerClassBuilderHelper
.mapOfPages([withoutBuilder]).buildLibraryForClass;
final generatedWith = routerClassBuilderHelper
.mapOfPages([withBuilder]).buildLibraryForClass;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit (low): this call builds the route with customRouteBuilder set outside any print-capturing zone, so the new production print fires for real here and the four-line WARNING => ... block is emitted into the test runner's stdout on every dart test run (confirmed locally — it shows up between the +5 and +6 lines). The two tests below already wrap mapOfPages in runZoned with a print override; wrapping this one the same way (or just reusing the captured-output helper) keeps the suite output clean.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants