history icon indicating copy to clipboard operation
history copied to clipboard

Support hashRoot in HashHistoryOptions

Open thejohnhoffer opened this issue 3 years ago • 11 comments

Closes issue #912

Calling createHashHistory with hashRoot="" replicates the lost hashType="noslash" from history@4. This PR is to address issue #8459 and issue #7703 of react-router.

Update

Until this PR is merged, this is still possible with [email protected] and use-hash-history.

I accomplish this with a function to convert between history.location and window.location

thejohnhoffer avatar Dec 09 '21 23:12 thejohnhoffer

As it stands now, I'm currently using this solution with a fork of history.

UPDATE: I'm currently using use-hash-history as a stand-in for this PR, following this template. It should be more maintainable than the fork.

thejohnhoffer avatar Dec 13 '21 15:12 thejohnhoffer

Hey @thejohnhoffer, thanks for this! A couple of quick things:

  • It looks like this PR is only showing your change to your fork's docs. Can you fix that for us?
  • Can you set the base branch to dev? We merge code changes there (I should probably update that default in GitHub)

Thanks!

chaance avatar Dec 17 '21 19:12 chaance

@chaance

Thank you for taking a look at this PR. Apologies for the sudden influx of code additions. To make testing my changes easier, I have created a new workspace for "use-hash-history". This will allow me to sort out the testing situation, my original PR apparently had some breaking changes.

There are a few paths forward... adding a "hashRoot" parameter to createHashHistory should certainly be possible without breaking the tests, but I also would like the "hashSlash" parameter for my own use case, which allows Hash states like #hash#part1#part2 to be treated as "/hash/part1/part2".

There's no reason we can't have both (as in, a simplified hashRoot parameter in history but also more complexity in my own custom Proxy package). If I have any support from maintainers, I'd ultimately like to work on a minimally invasive implementation of hashRoot that doesn't involve the creation of a whole new workspace and an npm package, but as a separate issue I need the more invasive changes for my current project.

Thank you so much for your time and consideration.

UPDATE:

This hashRoot feature is ready and is as "minimally invasive" as I can imagine.

thejohnhoffer avatar Dec 17 '21 23:12 thejohnhoffer

According to the tests, this is a non-breaking change with added support of hashRoot: "" as a replacement for the lost feature of hashType: "noslash". For my purposes, this PR is now ready to merge.

I'm happy to make any changes the maintainers decide should be made on this before merging. @chaance hopefully this PR is clearer to follow now, at only 35 lines of code (including tests).

thejohnhoffer avatar Dec 21 '21 21:12 thejohnhoffer

Hi @chaance -- I've again tested that the new and old tests all pass. My own alternative to this PR, use-hash-history, has a few users. I'm happy to continue maintaining my own package, but merging this PR would simplify the dependency tree of anyone who needs that feature.

I'm happy to simplify, remove, or re-write some of the tests I've created for this PR.

thejohnhoffer avatar Mar 10 '22 17:03 thejohnhoffer

Adding a +1 to request that this PR gets merged, thanks for creating this, @thejohnhoffer!

rahulgi avatar May 17 '22 01:05 rahulgi

+1 for this PR

marcinkowal2015 avatar Jun 17 '22 09:06 marcinkowal2015

Any chance to move this PR forward?

hakubo avatar Jul 07 '22 18:07 hakubo

@chaance how could we help moving this forward?

hakubo avatar Jul 12 '22 08:07 hakubo

@chaance "This branch has no conflicts with the base branch" - anything we could do?

hakubo avatar Sep 28 '22 12:09 hakubo

+1 for merging this PR

Mikilll94 avatar Oct 17 '22 18:10 Mikilll94