-
Notifications
You must be signed in to change notification settings - Fork 818
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
Updates to latest dependencies, and transpilation targets #1658
Changes from all commits
1653165
b2cd2ac
4fbb626
2276426
9666462
38f132b
59e7d4f
337c824
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
{"latestUrl":"https://storage.googleapis.com/workbox-cdn/releases/3.6.1"} | ||
{"latestUrl":"https://storage.googleapis.com/workbox-cdn/releases/3.6.2"} |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,12 +18,12 @@ module.exports = (packagePath) => { | |
`); | ||
|
||
return gulp.src(`${packagePath}/src/**`).pipe(babel({ | ||
only: /\.js$/, | ||
only: [/\.js$/], | ||
presets: [ | ||
['env', { | ||
['@babel/preset-env', { | ||
targets: { | ||
// Change this when our minimum required node version changes. | ||
node: '4.0', | ||
node: '6.0', | ||
}, | ||
}], | ||
], | ||
|
@@ -32,7 +32,7 @@ module.exports = (packagePath) => { | |
// are only included in our Rollup bundles once, even if they're used | ||
// in multiple source files. | ||
// See https://github.com/rollup/rollup-plugin-babel#helpers | ||
'transform-runtime', | ||
'@babel/transform-runtime', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BTW, if we're getting rid of the regenerator runtime polyfill, we may not need this at all anymore. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We still need it for the node libraries, since we're targeting node 6, and that doesn't support |
||
], | ||
})).pipe(gulp.dest(outputDirectory)); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
module.exports = () => process.platform === 'win32' ? | ||
'npm.cmd' : 'npm'; | ||
'npm.cmd' : 'npm'; |
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.
What's this being added for?
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.
Oh, now that I'm looking at the rest of the changes, I see what you're doing. But technically this isn't consistent with Google style because in some cases Google style recommends indenting 4 spaces (e.g. line continuations and stuff like that).
The google eslint package v0.10.0 adds an indentation rule more inline with what's recommended by Google JS style. It's probably worth just upgrading to that.
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.
As discussed in person, there are going to be a lot of additional files that need whitespace adjustment if we don't include this.
We're already using
"eslint-config-google": "^0.10.0"
as part of this PR.Let's do separate PR to remove this override and get all the whitespace consistent rather than including any more changes in this PR.
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.
SGTM