Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use global transform for babelify #22927

Merged
merged 6 commits into from Jun 20, 2019
Merged

Use global transform for babelify #22927

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a2a25d4
Attempt to use babelify.global.
Jun 19, 2019
20c1d0b
Oops, no ES6 in build-system/.
Jun 19, 2019
f839374
Simplify some code.
Jun 19, 2019
54ba76a
Set ignore for babelify in karma.conf.js.
Jun 19, 2019
d4db7d1
Add comment.
Jun 19, 2019
318db80
Fix comment.
Jun 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion build-system/tasks/karma.conf.js
View file
Open in desktop
Expand Up @@ -15,6 +15,7 @@
*/
'use strict';

const {devDependencies} = require('./helpers');
const {gitCommitterEmail} = require('../git');
const {isTravisBuild, travisJobNumber} = require('../travis');

Expand Down Expand Up @@ -68,7 +69,14 @@ module.exports = {
debug: true,
basedir: __dirname + '/../../',
transform: [
['babelify', {'global': isTravisBuild(), 'sourceMapsAbsolute': true}],
[
'babelify',
{
'global': isTravisBuild(),
Copy link
Contributor

@rsimha rsimha Jun 20, 2019 •
edited

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!)

Copy link
Author

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.

Copy link
Contributor

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.

Copy link
Author

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.

Copy link
Contributor

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.

'ignore': devDependencies(),
Copy link
Author

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.

'sourceMapsAbsolute': true,
},
],
],
// Prevent "cannot find module" errors on Travis. See #14166.
bundleDelay: isTravisBuild() ? 5000 : 1200,
Expand Down