Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

maybePop on web forgets query parameters in the url bar. #2085

Open
darkstarx opened this issue Nov 5, 2024 · 5 comments
Open

maybePop on web forgets query parameters in the url bar. #2085

darkstarx opened this issue Nov 5, 2024 · 5 comments

Comments

@darkstarx
Copy link

darkstarx commented Nov 5, 2024

Hello.

When I use @QueryParam on the root page, then push a new page and call context.maybePop, the url of the root page loses query parameters, although the state of the root page stays correct (parameters of the root page widget annotated with @QueryParam store correct data).

  @override
  List<AutoRoute> get routes
  {
    return [
      CustomRoute(path: '/',
        page: HomeRoute.page,
      ),
      CustomRoute(path: '/organizations/:orgId/projects/:projId',
        page: ProjectRoute.page,
      ),
    ];
  }


@RoutePage()
class HomePage extends ConsumerWidget
{
  final String? organizationId;
  final String? projectId;
  final String? search;

  const HomePage({
    super.key,
    @QueryParam('org') this.organizationId,
    @QueryParam('proj') this.projectId,
    @queryParam this.search,
  });

  @override
  Widget build(final BuildContext context, final WidgetRef ref)
  {  ...  }
}


@RoutePage()
class ProjectPage extends ConsumerStatefulWidget
{
  final String organizationId;
  final String projectId;

  const ProjectPage({
    super.key,
    @PathParam('orgId') required this.organizationId,
    @PathParam('projId') required this.projectId,
  });

  @override
  ConsumerState<ProjectPage> createState() => _ProjectPageState();
}

I push a new route like this:

    onTap: () => context.pushRoute(
      ProjectRoute(
        organizationId: project.organizationId,
        projectId: project.id,
      ),
    ),

What am I doing wrong?

@darkstarx
Copy link
Author

Ok, it looks like a bug in the StackRouter._push.
I don't know why it has been written here:

    if (result.continueNavigation) {
      _updateSharedPathData(
        queryParams: route.rawQueryParams,  //<<< HERE the new route's query parameters go into query parameters of previous route data.
        fragment: route.fragment,
        includeAncestors: true,
      );
      return _addEntry<T>(match, notify: notify);
    }

but it just erases the root page's query parameters and put there query parameters of the next page.

I can't find any settings to prevent such a behavior.

@darkstarx
Copy link
Author

@Milad-Akarie could you please help me to understand the purpose of this method?

  // should find a way to avoid this
  void _updateSharedPathData({
    Map<String, dynamic> queryParams = const {},
    String fragment = '',
    bool includeAncestors = false,
  });

It has been written many years ago in a big commit, so I can't figure out what was there.

@Milad-Akarie
Copy link
Owner

Still not sure about the implementation until today, shared path data are query params and the fragment.
Shared means that they are shared between all path segments.

@darkstarx
Copy link
Author

Ok, I see..
But sharing doesn't assume complete rewriting, write? May be we can somehow modify the logic of sharing this data?
We should leave all the data in lower routes and provide the direct access to the extended data from upper routes.

I'm actually not sure the lower routes should access route data from upper routes, but if it's really needed, we can write some class that could store autorelease data from upper routes when they are pushed in the stack, for example.

@darkstarx
Copy link
Author

I think that this sharing logic should be removed from the routing library, since that's the responsibility of the app logic.

If you get a deeplink, say https://root_page/middle_page/top_page?param=value, you will show the top_page route anyway, then you can share all it's data with other pages by the app mean if you really need it.
Now, if you pop the top_page, you will expect https://root_page/middle_page in the url bar, but not the https://root_page/middle_page?param=value, right? So, there is no information in any deeplink, which segments of the path need query parameters. That's not a purpose of URI and not a responsibility of the routing library.

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

No branches or pull requests

2 participants