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

fix(router): fix replaceState compatibility with jsdom #10001

Closed
wants to merge 2 commits into from

Conversation

jdufresne
Copy link
Contributor

Workaround upstream jsdom bug:
jsdom/jsdom#3504

The third argument to replaceState is optional, but provide it anyway for improved compatibility with all environments.

The call to replaceState was introduced in
bb7590a.

Workaround upstream jsdom bug:
jsdom/jsdom#3504

The third argument to replaceState is optional, but provide it anyway
for improved compatibility with all environments.

The call to replaceState was introduced in
bb7590a.
@changeset-bot
Copy link

changeset-bot bot commented Jan 29, 2023

🦋 Changeset detected

Latest commit: 9b63280

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@remix-run/router Patch
react-router Patch
react-router-dom Patch
react-router-dom-v5-compat Patch
react-router-native Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@MichaelDeBoey MichaelDeBoey changed the title Provide a URL to replaceState for compatibility with jsdom fix(router): fix replaceState compatibility with jsdom Jan 30, 2023
@machour machour requested a review from chaance January 30, 2023 07:00
@chaance chaance requested review from brophdawg11 and removed request for chaance January 30, 2023 20:06
@jdufresne
Copy link
Contributor Author

I was able to find a local workaround as well. So if you prefer to close, it is no problem for me and I'll continue with the workaround I have.

@brophdawg11
Copy link
Contributor

Thanks @jdufresne! Since this changes code, would you mind rebasing it and re-pointing it at the dev branch?

@jdufresne
Copy link
Contributor Author

Thank you for the review, but I'm going to close this PR in favor of persuing a fix upstream. React Router is using the History API as documented, so it shouldn't need to change. My local workaround is sufficient until JSDOM is fixed. Thanks again.

@jdufresne jdufresne closed this Jan 30, 2023
@jdufresne jdufresne deleted the replace-state branch January 30, 2023 23:32
@MichaelDeBoey
Copy link
Member

For people interested in the PR: jsdom/jsdom#3505

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants