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

[Docs]: why need localforage, match-sorter, sort-by in the example in the tutorial? #11099

Closed
electfreak opened this issue Dec 6, 2023 · 4 comments
Labels

Comments

@electfreak
Copy link

Describe what's incorrect/missing in the documentation

I was reading the tutorial. there is a command:

npm install react-router-dom localforage match-sorter sort-by

But there is no usage of these packages (except react-router-dom itself) further

@electfreak electfreak added the docs label Dec 6, 2023
@brophdawg11
Copy link
Contributor

They're used by the contacts module you have to copy/paste in:

👉 Copy/Paste the tutorial data module found here into src/contacts.js

@brophdawg11 brophdawg11 closed this as not planned Won't fix, can't repro, duplicate, stale Dec 7, 2023
@electfreak
Copy link
Author

@brophdawg11 it worth noting (for beginners) that these packages are not needed for react-router itself

@ShaneYu
Copy link

ShaneYu commented Feb 14, 2024

This needs addressing somehow, either the gist needs updating or the docs need to call it out more clearly that it's not needed by the react router itself.

Dependabot and other vulnerability scanners are screaming about prototype-pollution vulnerabilities in sort-by package, most of us included it because we were lead to believe it was needed by the tutorial.

Obviously we can just remove them, but might be a good idea to make it clearer to people.

@brophdawg11
Copy link
Contributor

We'd happily accept a PR to update the documentation!

adambirds added a commit to adambirds/react-router that referenced this issue Feb 22, 2024
This commit will help prevent people thinking that sort-by, localforage and match-sorter are actually required by react-router-dom by following the tutorial page. See remix-run#11099.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants