Update babel jest #32326
Update babel jest #32326
Conversation
💚 Build Succeeded |
| @@ -14,7 +14,7 @@ exports[`ScriptingHelpFlyout should render normally 1`] = ` | |||
| <EuiTabbedContent | |||
| initialSelectedTab={ | |||
| Object { | |||
| "content": <Unknown />, | |||
| "content": <ScriptingSyntax />, | |||
There was a problem hiding this comment.
jest improved React.Fragment support
jestjs/jest#5786
| @@ -6,7 +6,7 @@ exports[`FormLabel Basic initialization 1`] = ` | |||
| className="euiFormLabel" | |||
| id="ml_aria_label_undefined" | |||
| /> | |||
| <Component | |||
| <JsonTooltip | |||
There was a problem hiding this comment.
https://babeljs.io/docs/en/babel-plugin-transform-react-display-name
which is part of @babel/preset-react
supports adding displayName for React 16+ syntax
| @@ -313,7 +313,7 @@ exports[`UploadLicense should display a modal when license requires acknowledgem | |||
| maxWidth={true} | |||
| onClose={[Function]} | |||
| > | |||
| <Component | |||
| <_default | |||
There was a problem hiding this comment.
We use https://github.com/davidtheclark/focus-trap-react, which doesn't declare displayName. Seems it's a result of babel transpilation referring to export type.
| "@babel/parser": "^7.3.4", | ||
| "@babel/polyfill": "^7.2.5", |
There was a problem hiding this comment.
@spalger cannot find where added @babel/polyfill in kbn-test. Seems useBuiltIns: 'entry', doesn't work there. Does it?
There was a problem hiding this comment.
Hmm, well kbn-test is always executed in place, so it's probably borrowing the dependencies from kibana, and since useBuiltIns: 'entry' replaces @babel/polyfill with individual requires for core-js/modules/{feature} it's probably just borrowing the core-js version required by Kibana... Not the best strategy, but I guess that's why it works. That said, we don't actually require @babel/polyfill in kbn-test. @mistic is working on converting the root babel usages to babel 7 and at that time we'll be moving from babel-polyfill to @babel/polyfill in src/setup_node_env/babel_register/polyfill.js
There was a problem hiding this comment.
if package relies on environment to provide polyfilled runtime we might want to use useBuiltIns: false https://babeljs.io/docs/en/babel-preset-env#usebuiltins-false
There was a problem hiding this comment.
I'm a little concerned about running the tests with babel7 while the code is using babel6 via the optimizer.
Is there a good reason to upgrade jest to babel7 before the rest of the code base?
How are you feeling about the babel7 upgrade @mistic? Do you think it's too early to move the jest tests?
|
@spalger as you know I'm working in a separate branch trying to push babel 7 to be used everywhere in the whole project (upgrade babel6 and replacing typescript). I think maybe we can wait for the whole babel7 change? |
|
@spalger @restrry those changes don't conflict with the current work I'm doing. In fact they are part of them too. The only different thing is the part for Is up to you guys merge this or not! |
Summary
As per #31825 (comment)
after merging #32168 we are good to go
Note that we run all jest tests with
babel7from now on, whereas optimizer still uses babel6. Is there an issues to track migration progress?Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.[ ] This was checked for cross-browser compatibility, including a check against IE11[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support[ ] Documentation was added for features that require explanation or tutorials[ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers