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

history was missed as a peer dep #8198

Merged
merged 2 commits into from Nov 4, 2021
Merged

history was missed as a peer dep #8198

merged 2 commits into from Nov 4, 2021

Conversation

maraisr
Copy link
Contributor

@maraisr maraisr commented Nov 4, 2021

Check;

import { Action, Location, To, createPath, parsePath } from "history";

Currently not able to bundle react-rotuer-dom, as there is no dep on history in the file tree.

Worth noting seems to be only if pnpm is used, maybe just different linking strategy between it and yarn/npm.

@ryanflorence
Copy link
Member

It's a direct dependency now, maybe your package-lock needs to be updated?

@jacob-ebey
Copy link
Member

jacob-ebey commented Nov 4, 2021

It's a direct dependency now, maybe your package-lock needs to be updated?

We are importing "history" in "react-router-dom" without it existing directly in the package.json. It only exists in the package.json of "react-router". We will want to add it to "react-router-dom" 's package.json deps as well since it does in-fact directly import / re-export from history.

@maraisr
Copy link
Contributor Author

maraisr commented Nov 4, 2021

Ya in anycase, history is directly used by -dom — and not referenced in any of its package.json fields.

Ill let you guys decide if its a direct dep or a peerDep.

Just let me know when this is ready, and not sure when/if this is a regression — coz we've been on v6 alpha for quite a while with no issues.

@mjackson
Copy link
Member

mjackson commented Nov 4, 2021

History is a direct dependency. We need to do the same for react-router-native too.

@timdorr
Copy link
Member

timdorr commented Nov 4, 2021

History is a direct dependency. We need to do the same for react-router-native too.

Done. I'll wait for tests to pass and merge it in.

@timdorr timdorr merged commit fbe5b3f into remix-run:main Nov 4, 2021
@maraisr maraisr deleted the patch-1 branch November 4, 2021 21:42
@maraisr
Copy link
Contributor Author

maraisr commented Nov 4, 2021

Thanks for being so swift with this done @timdorr 🤝

Just wondering how long till this lands on npm?

mjackson pushed a commit that referenced this pull request Nov 5, 2021
Co-authored-by: Tim Dorr <git@timdorr.com>
@maraisr
Copy link
Contributor Author

maraisr commented Nov 8, 2021

Ah thanks for that one @timdorr — can see its now in v6.0.1.

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

Successfully merging this pull request may close these issues.

None yet

5 participants