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
Use global transform for babelify #22927
Use global transform for babelify #22927
Conversation
build-system/tasks/helpers.js
Outdated
@@ -404,6 +404,17 @@ function finishBundle(srcFilename, destDir, destFilename, options) { | |||
} | |||
} | |||
|
|||
/** | |||
* Returns array of relative paths to "dependencies" defined in package.json. |
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.
nit: Should this comment say "devDependencies"
?
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.
Oops, thanks.
{ | ||
// Transform "node_modules/", but ignore devDependencies (on Travis). | ||
'global': isTravisBuild(), | ||
'ignore': devDependencies(), |
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 think this is a performance optimization.
[ | ||
'babelify', | ||
{ | ||
'global': isTravisBuild(), |
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.
@choumx I just ran some unit tests locally, and I think this is now necessary for local runs too. Changing this line to true
, plus wiping the Travis cache might fix things. (I think!)
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.
Not sure, I actually didn't change this line.
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, what I meant was now that you're relying on this to transform node_modules
, it's necessary to enable global: true
for local testing and on Travis.
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 think https://travis-ci.org/ampproject/amphtml/jobs/548325896 is failing because of this too.
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.
The link you posted confirms my suspicion. The failing runs are all coverage runs, which uses a different babelify
config. I've sent out a fix in #22951, which I believe should handle this.
This reverts commit 6093a86.
* Attempt to use babelify.global. * Oops, no ES6 in build-system/. * Simplify some code. * Set ignore for babelify in karma.conf.js. * Add comment. * Fix comment.
Simplifies some code paths.
#22848 (comment)