Skip to content
This repository was archived by the owner on Dec 30, 2022. It is now read-only.

Migrate to React 16 #431

Merged
merged 3 commits into from
Oct 10, 2017
Merged

Migrate to React 16 #431

merged 3 commits into from
Oct 10, 2017

Conversation

mthuret
Copy link
Contributor

@mthuret mthuret commented Oct 6, 2017

Just to test with CI

Remaining known issues:

@mthuret mthuret changed the title wip Migrate to React 16 Oct 6, 2017
@algobot
Copy link
Contributor

algobot commented Oct 6, 2017

Deploy preview ready!

Built with commit aae7c11

https://deploy-preview-431--react-instantsearch.netlify.com

@algobot
Copy link
Contributor

algobot commented Oct 6, 2017

Deploy preview ready!

Built with commit 5911b20

https://deploy-preview-431--react-instantsearch.netlify.com

@mthuret mthuret requested review from vvo and Haroenv October 10, 2017 10:47
@@ -2,18 +2,19 @@
"private": true,
"devDependencies": {
"identity-obj-proxy": "3.0.0",
"jest-environment-jsdom-11.0.0": "^20.0.9",
Copy link
Contributor

Choose a reason for hiding this comment

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

this is now an external package is that it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry the documentation of jest says the default is jsdom, am I missing something?

Ok so reference is jestjs/jest#3655 and https://github.com/Micromeritics/jest-environment-jsdom-11.0.0

Let's watch those to be able to remove this code afterwards

@@ -23,7 +24,9 @@
"notify": false,
"moduleNameMapper": {
"\\.(css)$": "identity-obj-proxy"
}
},
"setupFiles": ["../../../../shim.js"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you can use <ROOT_DIR> smg here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could be better indeed :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vvo is here referring to the directory where the example package.json is, and therefore, can't work if shim.js is only at the very root of the project?

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed root_dir would be the example/package.json.

Also this seems temporary given comments in facebook/create-react-app#3199, just watch the issue and remove it afterwards

@@ -0,0 +1,4 @@
// see: https://github.com/facebookincubator/create-react-app/issues/3199
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Love when we do that :)

Copy link
Contributor

Choose a reason for hiding this comment

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

It's already fixed in Jest, just not released yet; see jestjs/jest#4545 (comment)

@vvo
Copy link
Contributor

vvo commented Oct 10, 2017

That's some serious chore/build stuff, much needed to ensure we are always up with the latest versions! GG

@vvo
Copy link
Contributor

vvo commented Oct 10, 2017

LGTM

@mthuret mthuret merged commit ed19d85 into master Oct 10, 2017
@mthuret mthuret deleted the migrate/react-16 branch October 10, 2017 15:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants