-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
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
chore(v2): upgrade to Babel 7 #1012
chore(v2): upgrade to Babel 7 #1012
Conversation
Deploy preview for docusaurus-preview ready! Built with commit 2667d3c |
d162371
to
e63446e
Compare
e63446e
to
fe25a31
Compare
v2/lib/webpack/base.js
Outdated
@@ -51,7 +51,7 @@ module.exports = function createBaseConfig(props, isServer) { | |||
.loader('babel-loader') | |||
.options({ | |||
babelrc: false, | |||
presets: ['env', 'react'], | |||
presets: ['@babel/env', '@babel/react'], | |||
plugins: [isServer ? 'dynamic-import-node' : 'syntax-dynamic-import'], |
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.
Haven't really tried this out but i think the dynamic import node babel plugin is not compatible with babel 7.
See airbnb/babel-plugin-dynamic-import-node#64
Happening in CRA as well
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.
@fiennyangeln Can you verify this? What do we do about this then?
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.
@yangshun yes, the PR for babel 7 upgrade for that plugin is still open. How about using https://www.npmjs.com/package/babel-plugin-transform-dynamic-import instead?
@@ -1,3 +1,4 @@ | |||
import '@babel/polyfill'; |
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.
Will it fails without this ? O.O
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.
yes!
@fiennyangeln Let us know when this is good to review again. |
b6221c9
to
85f9b0a
Compare
@yangshun please review 😄 |
@fiennyangeln The tests are failing 😓 |
…port" This reverts commit 9263aa6.
Ah sorry! I guess the changing of import order mess it all, reverted it back @yangshun |
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.
Thanks
Motivation
#984
(Write your motivation here.)
Have you read the Contributing Guidelines on pull requests?
Yes
(Write your answer here.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)
Related PRs
None
(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)