Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Changing 'react-router' imports to 'react-router-dom' in AdminUI #803

Merged
merged 3 commits into from
Mar 10, 2019

Conversation

nathsimpson
Copy link
Contributor

@nathsimpson nathsimpson commented Mar 10, 2019

Solving #802

It appears the error in issue #802 was due to a version mismatch between the beta versions of 'react-router' and 'react-router-dom' we are using in the Admin UI. As 'react-router-dom' depends on 'react-router', @mitchellhamilton suggested replacing any RR import with a RRD import.

Tested with the Todo and Blog demo projects, and all seems to work.

Note: also fixed a broken import in blog demo project

Copy link
Member

@emmatown emmatown left a comment

Choose a reason for hiding this comment

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

Could you also remove react-router from the root package.json?

@emmatown
Copy link
Member

More context about this:

What's happening here is that react-router-dom depends on react-router@^4.4.0-beta.6 which can resolve to react-router@4.4.0-beta.7 meaning react-router won't be hoisted so react-router and react-router-dom can have different router contexts. Since react-router-dom exports all of react-router's exports, we can use react-router-dom everywhere like in this PR to solve the problem.

@JedWatson JedWatson merged commit 73ea263 into master Mar 10, 2019
@JedWatson JedWatson deleted the Issue802 branch March 10, 2019 12:56
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants