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.

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

Babel 7.4 - babelHelpers.taggedTemplateLiteral() now showing up in ES6 build #33

Closed
moderndeveloperllc opened this issue Mar 20, 2019 · 7 comments

Comments

@moderndeveloperllc
Copy link
Contributor

commented Mar 20, 2019

@web-padawan Don't think this is an issue with your code, but it's going to cause problems since babel-helpers isn't included for module builds. Updating to Babel 7.4 includes what appears(?) to be a rather big change to how @babel/preset-env decides what modules to polyfill. This article touches on that. Even with useBuiltIns: false,, it still changes.

Basically, for some reason the ES6 build now thinks that all the tagged template literals (all web components) now needs a babelHelpers.taggedTemplateLiteral wrapper.

@moderndeveloperllc

This comment has been minimized.

Copy link
Contributor Author

commented Mar 20, 2019

Well, it looks like this is because Safari 12 has a particular bug with Template Literals. A fix went into compat-table that is causing this. Trying to figure out from the reporter if this is something that is a real issue or what.

BTW, you and the other component devs at @vaadin might want to peruse over this issue when you have time. As ES develops, having just a module/nomodule split like this project won't be as useful in the future as new proposals are added. That issue has lots of folks looking for answers.

@moderndeveloperllc

This comment has been minimized.

Copy link
Contributor Author

commented Mar 20, 2019

FYI, the Safari 12 bug is:

The bug causes:

function strings(array) {
   return array;
 }
 function getStrings() {
   return strings`foo`;
 }
 
 const ref = getStrings();
 
 // This should always be true
 // It may not be in Safari
 ref === getStrings();

Many libraries use the "always the same reference" property to do caching (via WeakMaps) of expensive work. If the references are not the same, then the caching is broken and the expensive work has to be redone. In some libraries, it can cause serious bugs.

I may try to work on a configuration-based workaround.

@web-padawan

This comment has been minimized.

Copy link
Owner

commented Mar 20, 2019

Thanks for the detailed research. Yes, my teammates have also faced this in Vaadin Flow 2.0 alpha, which is being rewritten to support ES modules. I will check the options to workaround this, it's unfortunate that Safari 12 needs some code that any other modern browsers don't 😞

@moderndeveloperllc

This comment has been minimized.

Copy link
Contributor Author

commented Mar 20, 2019

Always Safari isn't it? Going to be fun with Edge Chromium goes live too.

Right now I'm thinking a short term solution is just to ignore the bug. I use Safari 12 a bit and have never seen this issue. The reporter mentions this is a GC issue and "many" libraries have issues. Perhaps Polymer 3 & LitElement are ones that don't have issues. That would mean changing the es6 target list to exclude Safari 12. es6 would then build without any babelHelpers.

A longer term solution might be to have two babel-helpers files. One always loaded that includes helpers for all browsers, and one loaded with a nomodule tag that only includes older helpers. With two whitelists, this would allow for a slow accumulation of new proposal helpers/bug workarounds.

So in this case, taggedTemplateLiteral would be moved from helper-whitelist.js to a helper-whitelist-modern.js that's included in index.html for all requests.

@moderndeveloperllc

This comment has been minimized.

Copy link
Contributor Author

commented Mar 21, 2019

Well, this issue exposed another in preset-env where you can't target between versions for plugins. Because compat-table sees that Safari 12 can't to template literals properly, preset-env says everything below version 12 can't do it either. That means doing not Safari >= 12 in the browser target list won't work. You have to remove iOS and Safari all together which means other, non-helper, transforms won't occur.

I have upgraded the logic to create two babel-helper files. Do you want a PR on that? I've also upped the dependancies to Workbox 4 and a couple of the webpack plugins that had major number upgrades.

@web-padawan

This comment has been minimized.

Copy link
Owner

commented Mar 21, 2019

@moderndeveloperllc feel free to submit a PR if you have found a working solution 👍

@moderndeveloperllc

This comment has been minimized.

Copy link
Contributor Author

commented Apr 3, 2019

I think #34 is a decent solution. I've been using it for the past few weeks without issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.