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

[ALT-1508] fix: react-router import in gatsby spa to match the docs #930

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

ryunsong-contentful
Copy link
Contributor

Purpose

Update the react-router imports to match the Gatsby docs https://github.com/contentful/doc-app/pulls

@ryunsong-contentful ryunsong-contentful requested review from a team as code owners January 15, 2025 20:34
Copy link

vercel bot commented Jan 15, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nextjs-marketing-demo-bug-test 🔄 Building (Inspect) Visit Preview Jan 16, 2025 8:40pm
3 Skipped Deployments
Name Status Preview Updated (UTC)
experience-builder-test-app ⬜️ Ignored (Inspect) Jan 16, 2025 8:40pm
studio-nextjs-marketing-demo ⬜️ Ignored (Inspect) Jan 16, 2025 8:40pm
studio-react-vite-template ⬜️ Ignored (Inspect) Jan 16, 2025 8:40pm

Copy link
Contributor

@primeinteger primeinteger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The gatsby app's package.json should probably install react-router-dom (instead of react-router) as a dependency to match this change as well

@ryunsong-contentful
Copy link
Contributor Author

The gatsby app's package.json should probably install react-router-dom (instead of react-router) as a dependency to match this change as well

Sounds good, I did a mini quest and basically the summary is that react-router-dom is a super-set of react-router (basically has everything react-router has and more)

Copy link
Contributor

@primeinteger primeinteger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 👏

@ryunsong-contentful ryunsong-contentful force-pushed the ALT-1508-update branch 2 times, most recently from 3c2ea0a to 5ea7bcc Compare January 16, 2025 20:39
Copy link
Member

@elylucas elylucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it out locally and worked like a charm 🚀

@ryunsong-contentful ryunsong-contentful merged commit 55c606e into development Jan 17, 2025
17 checks passed
@ryunsong-contentful ryunsong-contentful deleted the ALT-1508-update branch January 17, 2025 00:03
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.

3 participants