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

Using async, await makes workbox without further transpilation break on Samsung Internet #812

Closed
HenrikJoreteg opened this issue Sep 20, 2017 · 7 comments
Assignees
Labels
Discuss An open question, where input from the community would be appreciated.

Comments

@HenrikJoreteg
Copy link
Contributor

See twitter discussion here: https://twitter.com/addyosmani/status/910631449753141253

@poshaughnessy noticed the starbucks PWA's SW isn't installing properly on Samsung Internet.

Turns out this is because of usage of async and await.

Library Affected:
all

Browser & Platform:
Samsung Internet

Issue or Feature Request Description:
Probably best to not use it in distributed versions if you expect people to be able to use it as is in Samsung Internet.

To repro, open https://preview.starbucks.com in inspect devices on samsung internet. It fails on line 80 of our SW which is the included Workbox code: https://preview.starbucks.com/serviceworker.js

WHich is the first usage of async/await.

@addyosmani addyosmani added Discuss An open question, where input from the community would be appreciated. P1 labels Sep 20, 2017
@NekR
Copy link

NekR commented Sep 20, 2017

I guess UCBrowser 5 latest UCBrowser which supports SW doesn't support async/await yet too.

@gauntface
Copy link

gauntface commented Sep 20, 2017

I'm in favor of dropping async and await usage in favor of promise chains. Seems like the sanest approach for developers using Workbox.

Tangental: we really should document our browser support.

@addyosmani
Copy link
Member

addyosmani commented Sep 21, 2017

I'm strongly in favor of us switching to just using promises. I would like to see if this is something we could do for the next release (2.0.2/2.1.0). @gauntface @jeffposnick wdyt?

It would unblock a few folks that are trying to use Workbox for production and as a forcing function would also help us figure out the browser support matrix for older versions of Chrome on mobile platforms.

Tentatively, latest version of mobile browsers - 2 sounds reasonable and if this included Samsung browser would catch some of their use of slightly older V8.

@jeffposnick
Copy link
Contributor

While it's still open for discussion, I wanted to take a look at the impact of changing our current Babel usage to include babel-preset-env with the following configuration:

const babelPlugin = babel({
  presets: [['env', {
    targets: {
      browsers: ['chrome >= 51'],
    },
    modules: false,
  }], ['babili', {
    comments: false,
  }]],
  plugins: ['external-helpers'],
});

It brings the final size of workbox-sw.prod.v2.0.1.js up by 1.8kb, to a total of 48879 bytes.

@jeffposnick
Copy link
Contributor

workbox-sw@2.0.2-rc1 is out (along with versions for our other modules), and it includes the transpilation.

It looks okay in my testing, but our naming convention for the built modules doesn't play nicely with the -rc1, so the one issue is that the output name is workbox-sw.prod.v2.0.2-rc1-2.0.2-rc1.0.js.

@HenrikJoreteg, if you could confirm that this works in your testing, we can tag it the official 2.0.2 release.

@jeffposnick
Copy link
Contributor

Just a heads-up: we're going to tag this code as the official 2.0.2 release tomorrow. If there are any last-minute incompatibilities, let us know.

@jeffposnick
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discuss An open question, where input from the community would be appreciated.
Projects
None yet
Development

No branches or pull requests

5 participants