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
Upgrade to Babel 7 #1511
Upgrade to Babel 7 #1511
Conversation
97a688a
to
7cc6bf5
Compare
242cb35
to
ba92df9
Compare
@@ -4,7 +4,7 @@ | |||
"description": "Backpack's stylesheets.", | |||
"main": "index.js", | |||
"scripts": { | |||
"build": "webpack" | |||
"build": "node build.js" |
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.
ad657d3
to
c344f9e
Compare
"jest": "TZ=Europe/London jest --coverage", | ||
"jest:update": "TZ=Etc/UTC jest --updateSnapshot", | ||
"jest:watch": "TZ=Etc/UTC jest --watch", | ||
"jest": "TZ=Etc/UTC jest --coverage", |
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.
One of the snapshots changed to use BST. I expect this is because something internally changed in Jest, so I set the timezone to UTC here.
this.listeners.forEach(cb => cb(...args)); | ||
} | ||
}; |
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 function stopped working without this, so I made it use arrow syntax.
@@ -24,7 +24,7 @@ exports[`BpkMapMarker should render correctly with a "arrowClassName" attribute | |||
<div | |||
className="bpk-map-marker__arrow custom-class-1 custom-class-2" | |||
> | |||
<Component /> | |||
<_default /> |
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'm struggling to find documentation for why this would change. It seems ok though so I'm not concerned. My gut feeling is that this is happening for components that don't have a name. In this case it's an imported icon.
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.
Yeah I would read that as "The default import from the package"
This comment has been minimized.
This comment has been minimized.
|
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 looks perfect! All seems to still work with this so happy to approve 👍 🎉
As well as Babel, this also upgrades Jest and Danger to the latest versions.
Getting
npm run build
to workBabel 7 doesn't work the same way for monorepos as Babel 6 did.
package.json
.As every folder in
packages/
has apackage.json
this means Babel no longer works. Babel 7.1+ provides an option here, namedrootMode: 'upward'
which reinstates the previous behaviour. To get this to work, I've added a newgulpfile.js
to every Gulp-using package that registers Babel with this mode enabled and then requires the ES6 gulpfile.For
bpk-stylesheets
that uses Webpack, I had to do a similar thing except with another option specified. This is detailed in thebuild.js
file inbpk-stylesheets
.Getting
npm run storybook
to workRemoved
/storybook/.babelrc
. Now inherits from the root config.Getting
npm test
to workDone. Some snapshots have updated as I also updated to the latest Jest and babel-jest.
Checking
base.js
is okbpk-stylesheets/base.js
was updated. To check this won't break things, I checked:✅ The
bpk-no-touch-support
class is added to non-touch browsers (tested in desktop Chrome)✅ The
bpk-touch-support
class is added to touch browsers (tested in mobile Safari)Helpful links