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

[Android] Deep link via intent creates multiple instances of MainActivity #7079

Closed
rreusser opened this issue Apr 20, 2016 · 11 comments
Closed
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@rreusser
Copy link
Contributor

rreusser commented Apr 20, 2016

We've configured <intent-filter> for our Android app as described in the docs so that our app can use oauth and handle deep links.

Expected behavior: intent can be received and handled by existing instance of MainActivity.

Observed behavior: the default configuration (android:launchMode="standard") of MainActivity causes every received intent to create an entirely new instance of MainActivity—which is received correctly by getInitialURL except now we have multiple MainActivity instances.

Version: 0.23.1, but nothing in 0.24 release notes suggests the behavior has changed.

See below for illustration. Here is the full example: https://github.com/rreusser/react-native-android-intent-issue

A solution: Adding android:launchMode="singleTask" to the MainActivity definition prevents multiple instances by routing the intent to the existing activity, but then getInitialURL is not adequate to handle deep links since it's no longer an initial URL. The solution in turn is for onNewIntent of MainActivity to send an 'openURL' event through RCTDeviceEventEmitter, which can be received by a (trivially) completed version of Linking.addEventListener for Android—which unless I'm mistaken is just a copy/paste job once the device event emitter is hooked up to onNewIntent.

This can be solved in userland, but the default launchMode of MainActivity leads to odd and problematic behavior that it's hard to imagine is what's usually desired. I'm glad to clean up our solution into a PR, but before going to the trouble wanted to confirm that this indeed a correct understanding of the problem and solution and that a fix isn't either intentionally relegated to userland or already on the way.

tl;dr: Unless I'm wrong, this all a long-winded way of describing what I believe would basically complete the implementation of Linking for Android.

Thanks!

example

@rreusser
Copy link
Contributor Author

rreusser commented Apr 20, 2016

Additional info from android <activity> docs. "standard" mode:

The system always creates a new instance of the activity in the target task and routes the intent to it.

and "singleTask" mode:

The system creates the activity at the root of a new task and routes the intent to it. However, if an instance of the activity already exists, the system routes the intent to existing instance through a call to its onNewIntent() method, rather than creating a new one.

@grabbou
Copy link
Contributor

grabbou commented Apr 20, 2016

Hey,

That's correct and expected (in a sense that getInitialURL no longer works). Also there's nothing wrong with the singleTask activity type AFAIK. In case of my app, it's something that can't be launched multiple times and still work properly.

As you can see from the links provided (https://github.com/facebook/react-native/blob/master/Libraries/Linking/Linking.js#L120):

Linking.addEventListener is not supported on Android

If you could submit a PR with that functionality it would be cool! (unless someone's already working on that at Facebook internally, cc: @mkonicek). The emitter is there, but the underlying code to emit these events is probably not there yet, so you might need to check that out as well.

We might consider changing the type to singleTask by default in generator, but I think documentation inside Linking section on Android should be enough and leave that extra bit of flexibility to users.

PS. I could help since I am using the similar setup, however I am using Branch.io behind the scenes and that's the place I've added the addEventListener integration to.

@rreusser
Copy link
Contributor Author

Makes sense. Yeah. The shortest path from onNewEvent of MainActivity to RCTDeviceEventEmitter will require some care, but it seems like a necessary connection that wouldn't hurt anything to exist. Agreed on leaving config to users and avoiding modifying the generator, if possible.

Remaining question is just whether there's other consequences of the behavior. (Effect on existing non-root activities?) My understanding is that most android apps consist of many activities so that this is a little smoother, but that RN heavily manages a single activity so that singleInstance is a better fit if using intents.

@rreusser
Copy link
Contributor Author

@grabbou — It's a shot in the dark, but I cleaned up our solution and submitted a PR. 🎉 At best it needs a couple tweaks, and at worst it's not a proper fit for this feature. More than a little skeptical that it was so simple. Which leads me to believe maybe it's not implemented for a reason. Feedback welcome though!

@skyride99
Copy link

I am using RN 23, and R 14.8.
I copied the changes into my branch of RN and it is not working.
When the app is launched from a link, the Linking.addEventListener for url is not triggered.

@rreusser
Copy link
Contributor Author

rreusser commented May 23, 2016

@skyride99 how are you building RN for android? I was rather surprised to learn (compared to iOS) that android RN libraries are distributed as a binary so that modifying the source in node_modules has no effect unless you follow the building from source instructions for android.

Glad to offer further thoughts, but that's the first thing that comes to mind. I got pretty frustrated before realizing the changes never had any effect!

@skyride99
Copy link

Thats the issue. It has to be rebuilt. Thanks for the tip. I appreciate the work. I hope the RN team will put in the next release.

@rreusser
Copy link
Contributor Author

rreusser commented May 24, 2016

@skyride99 It's also possible to do this strictly within the confines of your app. It requires adding onNewIntent to MainActivity.java and then creating your own module that connects to the DeviceEventEmitter. That glosses over lots of stuff, but long story short, it was wayy shorter to just include it in RN itself, hence the PR. It's a bunch of downloading, but I had no trouble at all (somewhat surprisingly) getting Android to build from source, so that'd be my first recommendation. I found the instructions very simple and effective. 👍

ghost pushed a commit that referenced this issue Jun 6, 2016
Summary:
Fixed #7118 by rreusser
Closes #7940

Differential Revision: D3391586

fbshipit-source-id: f7e572a91347fb0629602374cb6944eabf5b0e8f
bubblesunyum pushed a commit to iodine/react-native that referenced this issue Aug 23, 2016
Summary:
Fixed facebook#7118 by rreusser
Closes facebook#7940

Differential Revision: D3391586

fbshipit-source-id: f7e572a91347fb0629602374cb6944eabf5b0e8f
mpretty-cyro pushed a commit to HomePass/react-native that referenced this issue Aug 25, 2016
Summary:
Fixed facebook#7118 by rreusser
Closes facebook#7940

Differential Revision: D3391586

fbshipit-source-id: f7e572a91347fb0629602374cb6944eabf5b0e8f
@mkonicek
Copy link
Contributor

Hi there! This issue is being closed because it has been inactive for a while.

But don't worry, it will live on with ProductPains! Check out its new home: https://productpains.com/post/react-native/android-deep-link-via-intent-creates-multiple-instances-of-mainactivity

Product Pains has been very useful in highlighting the top bugs and feature requests:
https://productpains.com/product/react-native?tab=top

Also, if this issue is a bug, please consider sending a pull request with a fix.

@jslok
Copy link

jslok commented Dec 7, 2017

I see it looks like the issue was since fixed in PR #7940 but I am having the same problem on RN 0.51.0.

I have added android:launchMode="singleTask" to androidmanifest. When an app link is opened and my app is already open in the background, Linking.getInitialURL() just returns the original url used to open the app, not the new one. Do I need to do something else?

@jslok
Copy link

jslok commented Dec 7, 2017

Nevermind, I see you must use the listeners to handle the new url. Makes sense that getInitialURL wouldn't have been the answer.

componentDidMount() {
  Linking.addEventListener('url', this._handleOpenURL);
},
componentWillUnmount() {
  Linking.removeEventListener('url', this._handleOpenURL);
},

@facebook facebook locked as resolved and limited conversation to collaborators Jul 19, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

6 participants