React-native-navigation: [v2] Why we have to pass containerId?

Created on 17 Jun 2017  路  7Comments  路  Source: wix/react-native-navigation

Is containerId that required really?

Hi, I was working with version 2.0.49 and I read v1to2diff.MD file from beginning to end. Okay, v2 is way better than v1 API but why we still have to pass an argument something 'unpredictable'. I mean, think about that. I just wanted to go back from the current screen, why I have to pass containerId? You have the screen tree, can't you just assume 'okay this guy has not added an argument, so let's do this action from current screen' or at least can't we access last active screen's containerId (From Navigation class) ?

Actually, I'm gonna solve this problem easily but that will be a workaround, why would we need workarounds? Let's solve this together. Maybe I'll send a PR about that or at least a comment about 'how did I solve this problem.'

I'm waiting to hear from you guys. Thanks to all the contributors and Wix Engineering team for giving us this amazing library.

acceptediscussion v2

Most helpful comment

Okay, I get it but at least can't we just do containerIds predictable. I mean, now containerIds are string something like 'Container2'. My registered container name is 'ExampleScreen', so containerId will be 'ExampleScreen' too I think. I know, same screen can be open several times but at least we can do ExampleScreen1, 2, 3 etc.

The scenario is about Redux actually. I want to push a screen in an action but now, I have to know what is the current containerId. So, I have to store that information somewhere.

PS: Actually I think a bit, I don't really need to store containerId. I'm dispatching actions from screens, so I can pass containerId as an argument to my action. Btw, I understand you very well. As you said, this can cause problems but anyway, If I would be call Navigation.push() -or other functions- without containerId, that would be great.

All 7 comments

Thanks for trying out v2, keep in mind it's still in beta and a lot of features are still under development..

So the reason I require all commands to pass containerId is to solve a lot of bugs that was caused by bad architecture of v1 which basically caused by the async nature of JS<->native communication.
Relying on top most screen being the one you "expect" is a bad idea, and a lot of production bugs proved that to be true.
I can actually add a feature to implicitly get the containerId from the current top most screen, but I think it's a bad idea and will cause confusing bugs in production.

An example of a known bug in v1: you call navigation.pop(), but the user clicks on another tab before the command is processed by native, which causes a pop to happen on the wrong tab.
Same goes to any other command.

Okay, I get it but at least can't we just do containerIds predictable. I mean, now containerIds are string something like 'Container2'. My registered container name is 'ExampleScreen', so containerId will be 'ExampleScreen' too I think. I know, same screen can be open several times but at least we can do ExampleScreen1, 2, 3 etc.

The scenario is about Redux actually. I want to push a screen in an action but now, I have to know what is the current containerId. So, I have to store that information somewhere.

PS: Actually I think a bit, I don't really need to store containerId. I'm dispatching actions from screens, so I can pass containerId as an argument to my action. Btw, I understand you very well. As you said, this can cause problems but anyway, If I would be call Navigation.push() -or other functions- without containerId, that would be great.

I'm not keen on adding support for implicit containerId via current top screen. This is a giant hole in the design. I do think I have something more explicit in mind and probably allow you to do what you want.
The idea I had was something like

const topContainerId = await Navigation.findTopContainerId();

At first this seems a good middle ground. This way we don't have support for implicit containerId, but for some users, who don't care about those kind of bugs, provides the ability to query it in an explicit way, and due to it being async - also communicates the idea that you can never be sure what screen is on top.
This has a big problem though. What happens if there are more than 1 top screen? for example tabs. Also what happens to containers that are also on screen all the time? for example soon we will support custom containers in places like the top bar.

As you can see this presents not straightforward questions.

As you said, the best current solution is to pass this into the action. Also, the contract is that The containerId will never change for the life of the screen. So you can even save this.props.containerId to the store (redux/remx/mobx) and rely on it to never change per instance of a container.

I can add the ability to name the containerId according to the implementing class, this is possible, but it should still be unique per instance, and generated by the system. Maybe I can let you pass it to the commands.. this needs some more investigation.

First of all, thank you for the answer. Actually, as you approved, the best way is passing to actions. So, I guess we can close this issue. We don't really need any improvements, the current system can meet our needs. We just need more documentation about v2.

Thank you

@yusufyildirim, I agree. I'm sure it takes a bunch of effort working on the main code and documentation is the last thing to worry about. Nevertheless. i'm getting the impression V2 will come with quite a few changes. It would be nice to get an idea of what to expect with the migration.

@bogobogo 猬嗭笍

Was this page helpful?
0 / 5 - 0 ratings