slovnicki / beamer

A routing package built on top of Router and Navigator's pages API, supporting arbitrary nested navigation, guards and more.
MIT License
582 stars 128 forks source link

Deep Links are appended instead of replacing the route #654

Closed Bungeefan closed 3 months ago

Bungeefan commented 5 months ago

[!NOTE]
Disclaimer: As my time is currently severely limited, this report might be lacking some nicer or more appropriate phrasings and is likely not as polished as I would like it to be. I am sorry for this. This is also the reason why I am, at least at the moment, not able to provide a fix by myself and open a pull request.

Describe the bug

In bfe7ce3cc7c9f2c08bd62a4845da883a4213c1d0 all location usages were replaced with uri.toString() calls. However, this broke the deep linking feature and possibly even more. Now when triggering a deep link the link gets appended to the root URI (/), which results in weird and incorrect route URLs.

Example:

This happens because the absolute path check is broken in at least Utils#maybeAppend, however, a quick search through the code found more possible breakages due to the above-mentioned replacement action.

Beamer version: (e.g. v0.14.1, master, ...)

v1.6.0

To Reproduce

Steps to reproduce the behavior:

  1. Have a Flutter app that is configured for Deep Links.
  2. Trigger a deep link.
  3. Watch as the route URI is mangled with the deep link URI.

Expected behavior

Correctly checking for an absolute URI, so the deep link isn't incorrectly appended.

Device (please complete the following information):

Not applicable.

Additional context

The following code has been changed to just check if the whole URI starts with a slash instead of, as before, just the location (aka. the path part of a URI). As the old check only considered the path part, simply switching to uri.toString() broke it for every URI that has a scheme and/or authority part (which is now possible due to the new type): https://github.com/slovnicki/beamer/blob/bfe7ce3cc7c9f2c08bd62a4845da883a4213c1d0/package/lib/src/utils.dart#L250-L264

Therefore, the new absolute URI (example://app/settings) is simply appended to the existing URI (/)

Possible Fix

This seems to be a usable fix for this exact method (there are potentially more needing a similar fix) that allows deep links to work again, at the very least, in my short use case test:

static RouteInformation maybeAppend(
  RouteInformation current,
  RouteInformation incoming,
) {
  if (!incoming.uri.hasAbsolutePath) {
    return current.copyWith(
      location: current.uri.path.endsWith('/')
          ? '${current.uri}${incoming.uri}'
          : '${current.uri}/${incoming.uri}',
      state: incoming.state,
    );
  }
  return incoming;
}

Note: This would still be mishandling URIs that are not absolute but contain a scheme or authority, however, I am not sure if such a URI would be even valid!

MajdSallora commented 5 months ago

same issue here

i used this version and it worked for me

beamer: 1.5.6