Coder Social home page Coder Social logo

Comments (25)

satya164 avatar satya164 commented on May 2, 2024 21

Firstly, I don't think react-navigation should be responsible for preventing this on its own.

Native handles this automatically, and it's a common enough issue that react navigation should handle automatically IMO. I think it's unreasonable to manually handle it for every button in the app even if it was easy.

From my reply in the discussion on slack earlier:

I think we need several heuristics to block duplicate navigation actions:

  1. Both navigation actions should originate from the same screen (same key)
  2. Both navigation actions should be of the same type, e.g. both should be PUSH
  3. Both navigation actions should happen in short-interval, e.g. between the start and end of transition

Otherwise, it might block intentional navigations, e.g. user clicked another button or press back before the transition actually finished, or there is a RESET action after PUSH

from rfcs.

rbadr avatar rbadr commented on May 2, 2024 7

I completely agree with @satya164 , it's really a big issue for a lot of users and especially in production apps (users have different mobile phones, old/laggy ...), and react-navigation should be able to handle it (checking the route name and params before pushing a new route).

@brentvatne I have tried idempotent pushes but in dev mode it display the red screen (should not push route with duplicated key) when you push a route with the same key, and sometimes it mounts the same screen and unmout it quickly.
It also doesn't work with NavigationActions.navigate unfortunately.

We're using now debouncing on every button in our app to avoid duplicating screens (which is just a hack).

I hope that we can find a solution for this issue, and would love to help if necessary !

from rfcs.

brentvatne avatar brentvatne commented on May 2, 2024 6

@Vishal1419 - this.props.navigation.navigate({routeName: 'abc', key: 'whatever'})

from rfcs.

brentvatne avatar brentvatne commented on May 2, 2024 1

the drawer stack navigator should have an option to enable headers.

just put a stack inside or put a drawer inside a stack please open a rfc for this if you think it's important

routing from parent navigator to sub navigators without using initial route name and rather just using a key to reference the child navigator

see the rfc i mentioned

from rfcs.

j-mendez avatar j-mendez commented on May 2, 2024 1

This issue occurs if you rapidly press on something to navigate it will try to push a duplicate scene. To solve this keep track of the scene once you call navigate and make sure its not === to the same route onPress.

from rfcs.

dantman avatar dantman commented on May 2, 2024

I'm of two opinions.

Firstly, I don't think react-navigation should be responsible for preventing this on its own. It's an interesting extra feature, but I don't think react-navigation handling this should be the gold standard.
If you double submit an ajax call you get 2 HTTP requests, if you double submit a "ADD_ITEM" redux dispatch you add two items, if you double submit a form you get 2 HTTP POSTs, if you double submit an INSERT row for a sqlite database you get 2 new rows.
There are dozens of things other than a navigate dispatch you can do as part of a button press or other double submittable user action. And just about all of them will have similar issues of an unwanted extra being created.
As such I think the gold standard here is providing a way to make preventing double submit for any user action event easy. My preference would be creating a recommended library for this functionality (ie: blocking subsequent calls to a handler until the task started by the first one finishes). And providing an easy to use signal in react-navigation to indicate when a navigate action's task completes (ie: when all queued navigate actions have finished their transitions). Like a TransitionManager.isTransitioning() => bool, TransitionManager.runAfterTransition(cb), and/or TransitionManager.waitForTransitions() => Promise

Secondly however, if you do want to handle this automatically as an extra feature I have already come up with a fairly unobtrusive way of making navigate actions idempotent automatically.
It was in the second half of this comment: react-navigation/react-navigation#2578 (comment)
The basic idea is making navigate work like goBack where the key of the source (the route the action is being dispatched from, not the key of the route being created) is included and giving navigate a "exclusive push after" semantics (ie: fail with a warning if a route already exists after this route).

from rfcs.

Vishal1419 avatar Vishal1419 commented on May 2, 2024

When are you planning to release a new version which will have the capability to accept key in navigation function?

from rfcs.

evanjmg avatar evanjmg commented on May 2, 2024

key works for most cases, but if you navigate from the side drawer and then open the sidedrawer again and navigate to the same key it breaks :(. These bugs need to be fixed, but yeah let's keep the key solution! My workaround for this situation is the following:

debounce(() => {
    this.props.navigation.navigate(PAYMENT_DETAILS_VIEW)
  }, 750, { leading: true,  trailing: false })

from rfcs.

brentvatne avatar brentvatne commented on May 2, 2024

@evanjmg - can you post a demo of this on https://snack.expo.io? happy to investigate that

from rfcs.

radiodario avatar radiodario commented on May 2, 2024

I've been using the key solution, but i'm now getting an error on 1.2.1, which i think it's the same thing @evanjmg reported

should not push route with duplicated key fromMainSettings

push
    hashAssetFiles:105610:29
getStateForAction
    hashAssetFiles:110181:47
nav
    hashAssetFiles:104909:36
combination
    hashAssetFiles:92773:36
<unknown>
    hashAssetFiles:93532:31
dispatch
    hashAssetFiles:92542:36
<unknown>
    hashAssetFiles:99060:26
<unknown>
    hashAssetFiles:94050:26
navigate
    hashAssetFiles:105494:33
onPress
    hashAssetFiles:142823:27
touchableHandlePress
    hashAssetFiles:37322:45
_performSideEffectsForTransition
    hashAssetFiles:31600:34
_receiveSignal
    hashAssetFiles:31537:44
touchableHandleResponderRelease
    hashAssetFiles:31426:24
_invokeGuardedCallback
    hashAssetFiles:2707:23
invokeGuardedCallback
    hashAssetFiles:2681:41
invokeGuardedCallbackAndCatchFirstError
    hashAssetFiles:2684:60
executeDispatch
    hashAssetFiles:2767:132
executeDispatchesInOrder
    hashAssetFiles:2774:52
executeDispatchesAndRelease
    hashAssetFiles:6313:62
forEachAccumulated
    hashAssetFiles:6308:41
processEventQueue
    hashAssetFiles:6369:147
runEventQueueInBatch
    hashAssetFiles:6616:83
handleTopLevel
    hashAssetFiles:6620:33
<unknown>
    hashAssetFiles:6649:55
batchedUpdates
    hashAssetFiles:5748:26
batchedUpdatesWithControlledComponents
    hashAssetFiles:2865:34
_receiveRootNodeIDEvent
    hashAssetFiles:6648:50
receiveTouches
    hashAssetFiles:6662:249
__callFunction
    hashAssetFiles:2316:47
<unknown>
    hashAssetFiles:2145:29
__guard
    hashAssetFiles:2287:11
callFunctionReturnFlushedQueue
    hashAssetFiles:2144:19

from rfcs.

brentvatne avatar brentvatne commented on May 2, 2024

will need you to post a repro case to https://snack.expo.io @radiodario. this works as expected for me: https://snack.expo.io/SyeO1SwOG

from rfcs.

evanjmg avatar evanjmg commented on May 2, 2024

I have an example here when I go to B, I can't go back to A? Shouldn't it go back to it without using goBack or without the key? Without using key, I can go to a new view - but again that adds more to the stack. https://snack.expo.io/@evanjmg/idempotent-navigate . Unfortunately I cannot reproduce my sidebar bug - it's on a very large enterprise project, but I think nested navigators need more work.

Also, open the sidebar and click the one of buttons really fast. They will eventually stop working on slow taps, it's not an issue for without key.

from rfcs.

brentvatne avatar brentvatne commented on May 2, 2024

@evanjmg - I re-organized that example to be a bit more clear: https://snack.expo.io/SJxu8wv_f

you can't navigate to A with the key UNIQKEY! because there is no route A with key UNIQKEY!. the difficulty here is that you can't specify the key for the initial route and it's dynamically generated. so if you want to navigate back to it, you need to use goBack(). there are two upcoming changes that make it so you can easily use navigate for this:

but I think nested navigators need more work.

what do you mean? it's not possible to act on feedback like this, we need more specific information ;)

Also, open the sidebar and click the one of buttons really fast. They will eventually stop working on slow taps, it's not an issue for without key.

can't repro this, can you post a video? what does stop working mean?

from rfcs.

evanjmg avatar evanjmg commented on May 2, 2024

Some improvements:

  • the drawer stack navigator should have an option to enable headers.
  • routing from parent navigator to sub navigators without using initial route name and rather just using a key to reference the child navigator

from rfcs.

armanbabai avatar armanbabai commented on May 2, 2024

oh. not work on my device huawei y221 !

from rfcs.

Xenc5 avatar Xenc5 commented on May 2, 2024

@rbadr Are you able to use unique keys?

from rfcs.

rbadr avatar rbadr commented on May 2, 2024

@jeevantakhar I'm generating my own custom unique key

from rfcs.

jakeacooley avatar jakeacooley commented on May 2, 2024

I'm getting the same error as @radiodario. @brentvatne
Here is my StackNavigator which is nested inside of a TabNavigator:

import React from 'react';
import { TouchableOpacity, View, Text, Button } from 'react-native';
import { StackNavigator } from 'react-navigation';
import { Icon } from 'native-base';

import Profile from './Profile';
import Settings from './Settings';

import NavBar from '../../Components/NavBar';

import Colors from '../../../constants/Colors';

export default StackNavigator(
  {
    Profile: {
      screen: Profile,
      navigationOptions: ({ navigation }) => ({
        headerRight: (
          <TouchableOpacity
            onPress={() =>
              navigation.navigate({
                routeName: 'Settings',
                key: 'SettingsScreen'
              })
            }
          >
            <Icon name="ios-settings" style={{ paddingRight: 10 }} />
          </TouchableOpacity>
        ),
        headerTitle: navigation.state.routeName
      })
    },
    Settings: {
      screen: Settings,
      navigationOptions: ({ navigation }) => ({
        headerLeft: (
          <TouchableOpacity onPress={() => navigation.goBack()}>
            <Icon name="ios-arrow-back" style={{ paddingLeft: 10 }} />
          </TouchableOpacity>
        ),
        headerTitle: navigation.state.routeName
      })
    }
  },
  {
    navigationOptions: ({ navigation }) => ({
      // header: props => <NavBar {...props} {...navigation} />
      headerStyle: {
        backgroundColor: Colors.tintColor,
        shadowOffset: { width: 0, height: 0 },
        shadowRadius: 3,
        shadowOpacity: 0.3
      },
      headerTitle: props => <Text style={props.style}>{props.children}</Text>,
      headerTitleStyle: {
        flex: 1,
        fontFamily: 'bold',
        textAlign: 'center',
        letterSpacing: -0.2
      },
      headerLeft: <View />, // Empty View's to Center Title on Android
      headerRight: <View />
    })
  }
);

Error: should not push route with duplicated key SettingsScreen

from rfcs.

Rutulpatel7077 avatar Rutulpatel7077 commented on May 2, 2024

+1
should not push route with duplicated key

from rfcs.

ericvicenti avatar ericvicenti commented on May 2, 2024

This issue is fixed in recent versions of react-navigation. Just provide a unique key when you call navigation.navigate({ routeName: 'RouteName', key: 'unique-route' })

from rfcs.

Rutulpatel7077 avatar Rutulpatel7077 commented on May 2, 2024

I am using "react-navigation": "1.3.1", which version you are talking about ? @ericvicenti

from rfcs.

ericvicenti avatar ericvicenti commented on May 2, 2024

I think about 1.5. But you should switch to v2

from rfcs.

Rutulpatel7077 avatar Rutulpatel7077 commented on May 2, 2024

Thanks! It works!

from rfcs.

MacKentoch avatar MacKentoch commented on May 2, 2024

Still not fixed in V2 it can navigate twice to same scene even if using a unique key to navigate().

from rfcs.

brentvatne avatar brentvatne commented on May 2, 2024

@MacKentoch - post an issue with a repro

from rfcs.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.