-
Notifications
You must be signed in to change notification settings - Fork 80
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
moving to babel 7 #34
Conversation
I am having some issues with jest. @TrySound have you come across this before? |
It looks like I am running into this: jestjs/jest#6364 You got past it for react-beautiful-dnd ... |
Worst part is that this is now working on my local :| |
In the edge the babel-core bridge was needed to get jest to run |
rollup.config.js
Outdated
'@babel/preset-env', | ||
{ | ||
'targets': { | ||
'esmodules': true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every user who bundle their code will be forced to transpile this package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really want to start distribution of es6 code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have taken this out
rollup.config.js
Outdated
@@ -45,7 +45,19 @@ export default [ | |||
file: 'dist/memoize-one.esm.js', | |||
format: 'es', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be esm now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
commonjs plugin can be removed |
.babelrc
Outdated
] | ||
}, | ||
"test": { | ||
"plugins": [ | ||
"transform-es2015-modules-commonjs" | ||
"@babel/plugin-transform-modules-commonjs" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this env section with transform-modules-commonjs
can be removed since env is already includes this. The point is that rollup/webpack plugins are able to add modules
option internally. This allows to simplify config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will remove this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ll do it tomorrow
.babelrc
Outdated
"flow", | ||
["env", { "modules": false, "loose": true }] | ||
"@babel/preset-flow", | ||
["@babel/preset-env", { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may omit preset
and plugin
words to make this config shorter.
I recommend to use babel.config.js everywhere. It's more straightforward. |
Yes, jest will be able to upgrade only with major release. jest 24 will work with babel 7 |
No description provided.