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

es5 modules contain async/await #661

Closed
lukemelia opened this issue Jun 6, 2019 · 2 comments · Fixed by #684
Closed

es5 modules contain async/await #661

lukemelia opened this issue Jun 6, 2019 · 2 comments · Fixed by #684

Comments

@lukemelia
Copy link
Contributor

I was surprised to see that the es5 modules generated by orbit's build leaves async/await untouched, which means that orbit's es5 code will not run on IE <= 11, Mobile Safari <= 10.2, and Android <= 4.4.

I opened an issue about this in the build tool: glimmerjs/glimmer-build#60

With some help from @dgeb, we worked around this by transpiling orbit's es5 output further per our Ember app's config/targets.js, following the approach shown in this ember-auto-import issue comment: embroider-build/ember-auto-import#41 (comment)

@makepanic
Copy link
Contributor

This bit me too in an angular2 app. It's correctly importing the */dist/*/es5 assets but they contain async functions.

@dgeb
Copy link
Member

dgeb commented Aug 12, 2019

@lukemelia Embroider and ember-auto-import have recently been updated to transpile dependencies (see embroider-build/ember-auto-import#225). I believe this should allow you to remove your custom transpilation configuration for orbit.

With that said, there is definitely still an issue with the builds, which are neither fully modern nor fully ES5-compliant. It is indeed misleading (and wrong) for the ES5 builds to include async/await.

My current thinking is to provide the following targets in package.json:

  • modules - ES modules (ESM) + ES-latest language features (stage 4 and above)
  • modules:es5 - ESM + ES5

I believe this combination could work well both for developers who are open to transpiling (and optimizing) dependencies and for those who would prefer to avoid it.

I'm unsure if main should continue to combine ES5 + CJS, or should move to ES-latest + CJS. But I do think it needs to remain CJS until ESM is unflagged in Node.

Some related links:

Feedback welcome.

dgeb added a commit that referenced this issue Aug 15, 2019
This updates @glimmer/build to v0.10.2, which applies `for...of` and `async` / `await` transforms to ES5 builds. The ES5 builds available in each package's `dist/*/es5` dir should now be processed with these transforms. However, these builds will also need to be paired with the `regenerator-runtime` package to ensure that the `regeneratorRuntime` global is defined.

After some [discussion](#661 (comment)), @tchak and I have decided that the builds targeted by `main` and `module` should be ES-latest (rather than ES5). These builds are as small and performant as possible, and of course can be further processed with Babel if necessary.
dgeb added a commit that referenced this issue Aug 16, 2019
This updates @glimmer/build to v0.10.3, which applies `for...of` and `async` / `await` transforms to ES5 builds. The ES5 builds available in each package's `dist/*/es5` dir should now be processed with these transforms. However, these builds will also need to be paired with the `regenerator-runtime` package to ensure that the `regeneratorRuntime` global is defined.

Furthermore, @glimmer/build now ships an amd/es2017 distribution which is now used by default for tests. Future efforts should remove all use of amd, but it's convenient for tests right now.

After some [discussion](#661 (comment)), @tchak and I have decided that the builds targeted by `main` and `module` should be ES-latest (rather than ES5). These builds are as small and performant as possible, and of course can be further processed with Babel if necessary.
@dgeb dgeb closed this as completed in #684 Aug 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants