Upgrade to babel 7 #1325
Upgrade to babel 7 #1325
Conversation
Upgrade all packages to use babel 7. Moved the plugins into the root babel.config.json file. Use --root-mode upward to enable lookups into the root config file.
babel.config.json
Outdated
| "@babel/plugin-proposal-export-default-from", | ||
| "@babel/plugin-proposal-logical-assignment-operators", | ||
| "@babel/plugin-proposal-optional-chaining", | ||
| [ |
There was a problem hiding this comment.
This is great, but also, could we keep the plugins minimal and add them only as we need them?
How about we use the newest presets available?
Adding them all at once encourages usage of the features, which adds complexity to configuration. Some configs like don't work well with Flow. Not sure if it'll be an issue when using typescript.
There was a problem hiding this comment.
Agreed, we could add only the features we currently use
There was a problem hiding this comment.
These will probably be mostly redundant when using typescript.
babel deprecated the yearly presents, and these plugins were added using the babel-upgrade script. These are the latest versions of the plugins that we already had.
There was a problem hiding this comment.
If we look at baseweb, looks like there's minimal configuration by using the react preset. Perhaps we can try what's there first?
There was a problem hiding this comment.
My approach for tasks like this is to migrate first, and optimize later as it keeps diffs smaller and it more apparent on what the optimizations are.
Either way, this won't work until we fix the tests since the tests are referencing 'showcase' and it isn't being "compiled"
There was a problem hiding this comment.
trimmed to
"presets": [
"@babel/preset-env",
"@babel/preset-react"
],
"plugins": [
"@babel/plugin-syntax-dynamic-import",
"@babel/plugin-proposal-class-properties",
"@babel/plugin-proposal-export-default-from",
"@babel/plugin-proposal-optional-chaining",
"@babel/plugin-proposal-nullish-coalescing-operator"
]
There was a problem hiding this comment.
actually just removed dynamic-import as well.
| "babelrcRoots": [ | ||
| ".", | ||
| "./packages/*" | ||
| ], |
There was a problem hiding this comment.
With this, do we still need the rootMode option? Reading the Babel docs it seems to indicate that it would search from the roots upwards.
There was a problem hiding this comment.
ya.. we still need the rootMode otherwise babel wont look up the tree for the babel.config.js file.
Here are the docs for configuring babel in a mono-repo
https://babeljs.io/docs/en/config-files#monorepos
|
The tests are currently failing because showcase directly references react-vis and it isn't being "compiled". This will probably be fixed when we address the imports. I was hoping to fix this after converting to babel-7, but it looks like I'll have to fix it first. |
|
My approach for tasks like this is to migrate first, and optimize later as it keeps diffs smaller and it more apparent on what the optimaztions are |
|
PR to fix the imports #1326 |
| @@ -80,6 +97,7 @@ | |||
| "react-addons-test-utils": ">=15.4.2", | |||
| "react-dom": "^16.0.0", | |||
| "react-test-renderer": "^16.13.1", | |||
| "react-vis-showcase": "^0.1.0", | |||
There was a problem hiding this comment.
Hm why is this necessary? Won't it become a circular depedency?
There was a problem hiding this comment.
it is a ciruclar dependency. But that dependency already existed since the tests directly reference the components in showcase.
We can fix this when we refactor the tests.
#1328
| // 'index':path.join(__dirname,'..', 'src', 'index.js'), | ||
| // 'theme':path.join(__dirname,'..', 'src', 'theme.js') | ||
| } | ||
| modules: ['node_modules', path.join(__dirname,'..', 'react-vis', 'src')], |
There was a problem hiding this comment.
urgh, we should avoid custom node_modules resolutions, it's bound to cause headaches later (having this config along with babel config).
I thought yarn workspaces would allow us to avoid this.
There was a problem hiding this comment.
Completely agree.
yarn workspaces will definately help us with this, however there is a bunch of cleanup that has to be done first.
Next step (separate PR) is to clean up the imports in the react-vis, then we should be able to remove this module resolution.
Upgrade all packages to use babel 7.
Moved the plugins into the root babel.config.json file.
Use --root-mode upward to enable lookups into the root config file.