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

Deep Links are appended instead of replacing the route #654

Closed
Bungeefan opened this issue Jan 17, 2024 · 1 comment · Fixed by #660
Closed

Deep Links are appended instead of replacing the route #654

Bungeefan opened this issue Jan 17, 2024 · 1 comment · Fixed by #660

Comments

@Bungeefan
Copy link
Contributor

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 bfe7ce3 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:

  • App is in route /.
  • User triggers Deep Link example://app/settings
  • Behaviors before and after the mentioned commit:
    • Before: App routes to /setttings.
    • After: App routes to /example://app/settings.

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):

static RouteInformation maybeAppend(
RouteInformation current,
RouteInformation incoming,
) {
final incomingLocation = incoming.uri.toString();
if (!incomingLocation.startsWith('/')) {
return current.copyWith(
location: current.uri.toString().endsWith('/')
? '${current.uri}$incomingLocation'
: '${current.uri}/$incomingLocation',
state: incoming.state,
);
}
return incoming;
}

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
Copy link

MajdSallora commented Jan 21, 2024

same issue here

i used this version and it worked for me

beamer: 1.5.6

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 a pull request may close this issue.

2 participants