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

Add location.index #334

Closed
mjackson opened this issue Jul 18, 2016 · 15 comments
Closed

Add location.index #334

mjackson opened this issue Jul 18, 2016 · 15 comments
Labels

Comments

@mjackson
Copy link
Member

mjackson commented Jul 18, 2016

The whole idea behind location.key was to provide a key that could be used to uniquely identify a page's position in the history stack. Couldn't we just use a numeric index into that stack?

This means that on the initial page load location.key would be 0, which would potentially cause bugs for people doing if (location.key), but that's the only breaking change I can think of. Meanwhile, I think it may help us unlock a lot more functionality, especially around doing stuff like #36 which I've been thinking about for a while now...

@taion
Copy link
Contributor

taion commented Jul 18, 2016

On v3, the initial page load doesn't have a key at all.

More broadly, if you left the SPA and came back, you should expect a new key – not 0 again. Otherwise you'd get back the wrong location state.

@taion
Copy link
Contributor

taion commented Jul 18, 2016

You could do

`${sessionNonce}:${index}`

or just

`${nonce}:${index}`

though.

That would give you the best of both worlds.

@mjackson
Copy link
Member Author

On v3, the initial page load doesn't have a key at all.

Ya, this would fix that because you can just default to 0 now.

or just
${nonce}:${index}

I like that idea. We could leave it up to people to generate a nonce for themselves to do things like restoring scroll position. If we provide the right primitives, people are empowered to do stuff like this.

@mjackson
Copy link
Member Author

In fact, maybe we just introduce location.index...

@taion
Copy link
Contributor

taion commented Jul 18, 2016

You need the nonce for correct state semantics even without transient stuff like scroll. Suppose:

  1. You're navigating around www.foo.com.
  2. Then you go to www.bar.com.
  3. Then you go back to www.foo.com via a PUSH.

For (3), history internally needs new keys; if not, at best you're going to clobber the location state associated with the locations in (1).

@mjackson
Copy link
Member Author

You need the nonce for correct state semantics even without transient stuff like scroll.

Ah, yes. That makes sense. Thanks for helping me think through this.

@mjackson mjackson changed the title Use numeric location.key Add location.index Jul 18, 2016
@ricardosoeiro
Copy link

ricardosoeiro commented Jul 21, 2016

This would be an awesome addition, it would allow us to:

  • distinguish back from forward transitions, to use different animations
  • understand when the user navigates past the application entry point, to warn the user before leaving

@iotch
Copy link

iotch commented Jul 21, 2016

How about Date.now()? It's very unlikely that there will be a transition with the same index. And it's comparable.

@taion
Copy link
Contributor

taion commented Jul 21, 2016

No – location.index needs to be consecutive series of integers, to allow the full set of functionality described in the OP.

@tommoor
Copy link

tommoor commented Aug 10, 2016

Hey guys, any thoughts on a timescale for this one? I'm still trying to implement proper back / forward buttons and somewhat blocked by this. Is the final conclusion to add the integer after the nonce as the location key?

@mjackson
Copy link
Member Author

mjackson commented Sep 2, 2016

Turns out we can't actually do this in browsers without crazy, crazy hacks, which I'm not willing to introduce. We'll expose canGo in memory history in v4 for people creating their own browsers (see #359).

@mjackson mjackson closed this as completed Sep 2, 2016
@OliverJAsh
Copy link
Contributor

OliverJAsh commented Aug 16, 2017

distinguish back from forward transitions, to use different animations

I was able to achieve this by keeping track of location.keys whenever they changed. We know a pop action is back if the second to last key in the tracked history state is equal to the new key.

  componentWillReceiveProps(nextProps: Props) {
    const { location } = this.props;
    const hasRouteChanged = nextProps.location !== location;
    if (hasRouteChanged) {
      switch (nextProps.location.action) {
        case 'PUSH': {
          this.historySinceEnter.push(nextProps.location.key);
          break;
        }
        case 'POP': {
          // Pop state changes fire for all navigations (back and forwards). We use the saved
          // history state to distinguish between backward and forward navigations.
          const lastHistoryIndex = this.historySinceEnter.length - 1;
          const previousLocationKey = this.historySinceEnter[lastHistoryIndex - 1];
          const isBack = previousLocationKey === nextProps.location.key;
          if (isBack) {
            this.historySinceEnter.pop();
          } else {
            this.historySinceEnter.push(nextProps.location.key);
          }
          break;
        }
      }
    }
  }

I could be missing some edge cases though. Thoughts?

@TimonVS
Copy link

TimonVS commented Sep 4, 2017

@OliverJAsh How do you get access to nextProps.location.action? It's undefined for me. Could you provide a complete example maybe?

@OliverJAsh
Copy link
Contributor

I'm using React Router 2.1, and whatever version of History is used by that.

@TimonVS
Copy link

TimonVS commented Sep 4, 2017

Ah, that makes sense. Thanks for the info!

@lock lock bot locked as resolved and limited conversation to collaborators Jun 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

7 participants