feat(atom-with-hash): allow optional setHash

[V-Mann-Nick]

Problem

Changing the hash using window.location can be in conflict with the next router. To reproduce this issue within a next app:

1. Use atomWithHash somewhere in the app using { replaceState: true }
2. Navigate to the page with atomWithHash (using next router)
3. Trigger a change that will invoke setting the hash via atomWithHash
4. Navigate in browser going back in history
5. Try and navigate in browser going forward
6. Will behave unexpectedly changing the URL, but not changing the page

This might also be unexpected behavior from the perspective of the next router. I'm not entirely sure. But I can imagine other routers possibly also having problems with this.

Solution

Changing the setHash function fixed this issue so my proposed solution using this commit's changes are:

import Router from 'next/router'
import { atomWithHash } from 'jotai-location'

atomWithHash('key', 'default', {
  subscribe: (callback) => {
    Router.events.on('hashChangeComplete', callback)
    return () => {
      Router.events.off('hashChangeComplete', callback)
    }
  },
  setHash: (searchParams) => {
    Router.replace({ hash: searchParams.toString() })
  },
})

What I don't like about these changes

I'm not entirely happy with this API as using replaceState with setHash makes replaceState unused. But for now I wanted to make minimal changes.

what do you think?

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 864ff8a:

Sandbox	Source
React	Configuration
React Typescript	Configuration

[dai-shi]

Hi, thanks so much for making suggestions for this library!
I wasn't aware the issue with nextjs. Wonder if there are some other nextjs users out there.
Thought replaceState option was suggested by one of nextjs users.
Anyway, if it doesn't work, we are happy to add an option to support such cases.

But for now I wanted to make minimal changes.

I think we can be creative to have new ideas.
Maybe:

setHash?: 'default' | 'replaceState' | ((params: URLSearchParams) => void)

params can be string.

We still could have replaceState option with a deprecation message for a while before completely removing it.

Would be nice to know how you and others think.

Let me pin some users who has contributed to atomWithHash before in https://github.com/pmndrs/jotai/pulls?q=atomWithHash.
@notsidney @austinwoon @jakkku

[V-Mann-Nick]

I think I've figured out what might be going wrong with the Next router when setting the hash in the default setHash.

When replaceState = false

Here the hash is set with window.location.hash = hash which will trigger the hashchange event. I would assume that the next router is listening to that and in this case it works as expected.

When replaceState = true

Here the hash is set with window.history.replaceState. This unfortunately does not cause a hashchange event as described here (this is the docs for pushState but the behavior is the same as this Stackoverflow answer says).

I would assume it has something to do with this behavior. Not sure.

Regarding your API proposal

I think that would make sense and would make the connection between setHash and replaceState more clear. If others like this, I can make the appropriate changes.

Regarding my example

The above example using with the Next router was incomplete and will break in case searchParams.toString() is empty.
My altered example with the next router would be:

import Router from 'next/router'
import { atomWithHash } from 'jotai-location'

atomWithHash('key', 'default', {
  subscribe: (callback) => {
    Router.events.on('hashChangeComplete', callback)
    return () => {
      Router.events.off('hashChangeComplete', callback)
    }
  },
  setHash: (searchParams) => {
    const hash = searchParams.toString()
    Router.replace({ pathname: Router.pathname, query: Router.query, hash })
  },
})

[austinwoon]

I think I've figured out what might be going wrong with the Next router when setting the hash in the default setHash.

When replaceState = false

Here the hash is set with window.location.hash = hash which will trigger the hashchange event. I would assume that the next router is listening to that and in this case it works as expected.

When replaceState = true

Here the hash is set with window.history.replaceState. This unfortunately does not cause a hashchange event as described here (this is the docs for pushState but the behavior is the same as this Stackoverflow answer says).

I would assume it has something to do with this behavior. Not sure.

Regarding your API proposal

I think that would make sense and would make the connection between setHash and replaceState more clear. If others like this, I can make the appropriate changes.

Regarding my example

The above example using with the Next router was incomplete and will break in case searchParams.toString() is empty. My altered example with the next router would be:

import Router from 'next/router'
import { atomWithHash } from 'jotai-location'

atomWithHash('key', 'default', {
  subscribe: (callback) => {
    Router.events.on('hashChangeComplete', callback)
    return () => {
      Router.events.off('hashChangeComplete', callback)
    }
  },
  setHash: (searchParams) => {
    const hash = searchParams.toString()
    Router.replace({ pathname: Router.pathname, query: Router.query, hash })
  },
})

Nothing much to add but just that this checks out. Looks like NextJS's router logic subscribes to their own hashChangeComplete event when changes are made to circumvent the missing event on replaceState. Router.replace emitshashChangeComplete after changing the hash.

I like dai-shi's signature and his idea of keeping replaceState as a separate option before removing it.

[dai-shi]

Sounds good.
@V-Mann-Nick Would you like to work on it?

[V-Mann-Nick]

Sure will do. I will make the edits to this PR by end of week.

[V-Mann-Nick]

@dai-shi I've made the changes as proposed.

[dai-shi]

Looks very good!

[dai-shi]

Published: https://www.npmjs.com/package/jotai-location/v/0.3.0

Thanks for your contribution! Please feel free to work on any other improvements in coding, making examples, and documenting, if you are interested.