upgrade to babel 6 by erwinmombay · Pull Request #9775 · ampproject/amphtml

erwinmombay · 2017-06-07T18:06:09Z

erwinmombay · 2017-06-07T18:09:17Z

.babelrc

+    ["env", {
+      "loose": true,
+      "targets": {
+        "browsers": ["last 2 versions", "safari >= 7"]


@jridgewell please correct this if needed

cc @cramforce

@jridgewell im guessing you want this dynamic based on the original issue, depending on if we're in test or dev mode etc.

Yes, CI would get this string (safari >= 9, though is what I think we support), and in dev testing we would use the latest Chrome

jridgewell

This is awesome that it worked so seamlessly!

jridgewell · 2017-06-07T19:00:09Z

extensions/amp-access/0.1/access-expr-impl.js

+        }
+        _parseError.prototype = Error;
+
+        throw new _parseError(str, hash);


What's this change for?

babel6 was erroring out on some label statements in the generated jison file (because of use strict + label usage), i tried upgrading jison and regenerated the files but it was still failing and had to add the "transform-remove-strict-mode" plugin to get it to work.

specifically its this line https://github.com/ampproject/amphtml/pull/9775/files#diff-8d20dae7ae84978e5dba004cfa21ff72L304

Odd, that line shouldn't throw. 😞

jridgewell

Please log the built file sizes (both build and dist). Doesn't need to be part of the PR, just a comment is fine.

jridgewell · 2017-06-07T19:24:28Z

gulpfile.js

  }

+  var browsers = [
+    process.env.TRAVIS ? "safari >= 9" : "Last 1 Chrome versions"


This needs the "last 2 versions" in Travis.

cramforce · 2017-06-07T19:38:19Z

Nice!

lannka · 2017-08-22T21:08:27Z

I ended up into this issue when last time trying to do babel6: babel/babel#4480
custom-element cries illegal constructor bla blaa

I suppose this has been fixed by one of the plugin?

erwinmombay · 2017-09-11T18:18:51Z

test/size.txt

@@ -1,98 +1,98 @@
      max   |         min   |        gzip   |   file
      ---   |         ---   |         ---   |   ---
-   290 kB   |    31.63 kB   |    12.49 kB   |   alp.js / alp.max.js


@jridgewell seems to be some hefty increases on the babel sizes, lmk if theres any option that i missed turning on/off that would have done this.

it doesnt really affect the files we serve built from CC.

This is because of babel-polyfill which is MASSIVE

erwinmombay · 2017-09-11T18:20:31Z

extensions/amp-viewer-integration/0.1/test/integration/test-amp-viewer-integration.js


      const port = new WindowPortEmulator(
-        this.win, viewerOrigin);
+        window, viewerOrigin);


@chenshay can you double check that this is ok

jridgewell · 2017-09-11T20:46:04Z

package.json

    "prepend-v0": "gulp prepend-global --prod node_modules/AMP_CONFIG.json --local --target dist/v0.js && gulp prepend-global --prod node_modules/AMP_CONFIG.json --local --target dist.3p/current-min/f.js"
  },
  "dependencies": {
+    "babel-polyfill": "6.26.0",


Hopefully this isn't needed.

unfortunately it is needed for regenerator-runtime

I think regenerator only needs some symbol polyfills, which should be much smaller than the entire babel-polyfill.

@jridgewell my expectation was babel-preset-env would have figured that out and added the polyfill but all the issues i found told me to include babel-polyfill. is there a specific package i can include that will allow me to remove babel-polyfill?

We can either drop regenerator (loose generator support in Safari 9), or dig through the regenerator runtime code to see what we need.

@jridgewell got it. do you mind if i create a separate issue/pr for that and merge this?

jridgewell · 2017-09-11T20:46:47Z

test/size.txt

@@ -1,98 +1,98 @@
      max   |         min   |        gzip   |   file
      ---   |         ---   |         ---   |   ---
-   290 kB   |    31.63 kB   |    12.49 kB   |   alp.js / alp.max.js


This is because of babel-polyfill which is MASSIVE

erwinmombay · 2017-09-13T20:09:53Z

follow up issue here #11291

rsimha · 2017-09-15T04:06:25Z

@erwinmombay @danielrozenberg all yarn dependencies were removed in #11279. Looks like this PR wasn't rebased properly, and has re-added yarn.lock. It'll need to be removed again.

erwinmombay requested review from jridgewell and rsimha June 7, 2017 18:06

googlebot added the cla: yes label Jun 7, 2017

erwinmombay force-pushed the babel-6 branch 2 times, most recently from cacebf2 to 26926aa Compare June 7, 2017 18:08

erwinmombay commented Jun 7, 2017

View reviewed changes

jridgewell approved these changes Jun 7, 2017

View reviewed changes

jridgewell requested changes Jun 7, 2017

View reviewed changes

jridgewell reviewed Jun 7, 2017

View reviewed changes

erwinmombay force-pushed the babel-6 branch from ec4b8c3 to d59bd51 Compare June 7, 2017 19:31

erwinmombay force-pushed the babel-6 branch from d59bd51 to 8582d90 Compare June 9, 2017 17:43

erwinmombay force-pushed the babel-6 branch from 8582d90 to 8494a85 Compare August 9, 2017 17:55

erwinmombay force-pushed the babel-6 branch from 8494a85 to 9ae62ac Compare August 21, 2017 20:20

erwinmombay requested review from aghassemi, alanorozco, chenshay, cramforce, dvoytenko, lannka and zhouyx as code owners August 21, 2017 20:20

erwinmombay force-pushed the babel-6 branch from 9ae62ac to 0316956 Compare August 21, 2017 20:42

erwinmombay force-pushed the babel-6 branch 6 times, most recently from a96a5b4 to c3537aa Compare August 30, 2017 20:21

erwinmombay force-pushed the babel-6 branch from 378ed0c to a0d526e Compare August 31, 2017 21:07

erwinmombay commented Sep 11, 2017

View reviewed changes

erwinmombay force-pushed the babel-6 branch from 1f55f26 to c22f11b Compare September 11, 2017 18:20

erwinmombay commented Sep 11, 2017

View reviewed changes

jridgewell reviewed Sep 11, 2017

View reviewed changes

ampprojectbot added this to the Pending Triage milestone Sep 12, 2017

jridgewell approved these changes Sep 13, 2017

View reviewed changes

erwinmombay added 16 commits September 13, 2017 12:37

upgrade to babel 6

5a4ef9b

remove loose mode in dep check

308a049

make browser env support dynamic

41df3fe

temp

99994d9

add regenerator runtime

b8dc029

stuff

07ddbca

add babel-polyfill to tests

a07db85

temp

973cff3

recompile sha384-generated.js

ed19499

temp

c7f4c14

temp

09fd93b

remove ignore

cc63050

temp

888b2ae

recompile jison built files

2c06c41

fix window references

44d04b4

new size.txt

4eddc11

erwinmombay mentioned this pull request Sep 13, 2017

remove babel-polyfill dependency #11291

Closed

erwinmombay force-pushed the babel-6 branch from c22f11b to 4eddc11 Compare September 13, 2017 20:08

erwinmombay merged commit 712cff9 into ampproject:master Sep 13, 2017

This was referenced Sep 15, 2017

Delete yarn.lock and update package-lock.json #11302

Merged

Replace npm with yarn #11332

Merged

rsimha mentioned this pull request Oct 5, 2018

🏗✨ Upgrade to Babel 7 #18574

Merged

Conversation

erwinmombay commented Jun 7, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jridgewell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erwinmombay Jun 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jridgewell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cramforce commented Jun 7, 2017

Uh oh!

lannka commented Aug 22, 2017

Uh oh!

erwinmombay Sep 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erwinmombay Sep 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erwinmombay commented Sep 13, 2017

Uh oh!

rsimha commented Sep 15, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

erwinmombay Jun 7, 2017 •

edited

Loading

erwinmombay Sep 11, 2017 •

edited

Loading

erwinmombay Sep 13, 2017 •

edited

Loading