[BUGFIX lts] Backport dynamic Ember compilation from 3.13 #18584
Conversation
Preparation/Cleanup for dynamic builds. Cherry-picked from PR emberjs#18206
Right now, `node-module` is a precompiled package, but that makes it harder to get it to properly integrate with dynamic builds. This changes it to be a standard package that goes through the build process like normal. Cherry-picked from PR emberjs#18207
This PR sets up Ember to do a two phase build: * **Phase 1, prepublish:** Run Typescript and Rollup on the main Ember packages, and strip out canary-features. Publish these packages, along with the dependencies for Ember, in the `dist/` folder. * **Phase 2, in addon:** Run Babel transpilation using the consumer's `ember-cli-babel` and Babel configuration on the dist packages and dependencies, and bundle up the final `ember.js` file to serve to apps. This also includes `debug` flags and, in theory, svelting. Two major changes that will occur because of this: 1. We will no longer be distributing `ember.prod.js`, `ember.debug.js`, `ember.min.js`, or `ember-testing.js`. These files existing may be something that people rely on, and the packages that _are_ distributed aren't quite ready to build in a "normal" way, using webpack or another bundler. 2. We will no longer be _building_ `ember.prod.js`, `ember.min.js`, or `ember.debug.js` at all, only a single `ember.js` file. We no longer need separate builds, because the environment settings of the build will handle the differences for us. We _are_ continuing to distribute a pre-built version of the `ember.js` and `ember-testing.js` files. These are used only in the case where the build targets match the default development build targets for Ember apps, providing a small optimization for users' dev workflow. Unfortunately, until we disentangle `ember-cli` from the implementation details of `ember-source` we cannot convert Ember to _build_ like a normal addon locally. Using the `EmberAddon` class blows up in many small ways. [BUGFIX lts] Updates based on feedback Cherry-picked from PR emberjs#18208
We had a bug where if you had additional targets added, they would not be checked, we were just checking that all the prebuilt targets were included. This adds a check for length to ensure all the targets are there and no additional ones are. Fix lint Cherry-picked from PR emberjs#18281
Helpers were no longer being externalized in our builds, which adds a significant amount to the final payload. This adds them back into the build. Also fixes an issue where the template compiler was not being included in the vendor folder for Ember. ensure async helpers are inlined in dev Cherry-picked from PR emberjs#18291
The EMBER_ENV variable doesn't get set until addon hooks actually start running, so we weren't correctly determining the environment. Cherry-picked from PR emberjs#18304
Ensure `__ARGS__` is only forwarded once in prod Cherry-picked from PR emberjs#18557
timiyay
force-pushed
the
backport/3-12-ember-dynamic-compilation
branch
from
67a6e7e to
19ababe
Compare
|
The last commit timed out in CI. Closing and reopening this PR, as I was told that's one way to trigger a rebuild of CI. UPDATE: yup that worked https://travis-ci.org/emberjs/ember.js/builds/617952379 |
timiyay
changed the title
|
@pzuraq backport commits are done. I squashed each PR into a single commit. Other than that, it was bog-standard cherry picks all the way. I'm going to continue some smoke testing against our apps, and also figure out whether the CI failures are genuine, or just Browserstack timeouts. |
|
What are the advantages of these features? I haven't heard of them before now |
|
This isn't a feature, so much as a massive bugfix. Before this change, we were precompiling, bundling, and shipping Ember. Due to native class interop issues, we had to try to match the target browsers of the consumer. We tried by shipping 2 builds simultaneously, one for modern browsers, one for legacy browsers, but it resulted in a large number of bugs. This change compiles Ember dynamically, like any other Ember addon, using |
|
FWIW @kanongil's recent PR #18576 fixes the most urgent and impactful regression - insta-crashing caused by leftover These dynamic build features are definitely an improvement but, if landing this backport proves challenging, we'd be perfectly happy using #18576 via 3.12 LTS, and wait until our next LTS bump to consume these dynamic builds. Our focus is much narrower than all the problems this backport will fix, though. |
|
There are two reasons why I want to land this:
You got in 90% of the way @timiyay, I haven't had a chance to review just yet but it looks good, and I'll figure out what's going on with the IE11 tests 😄 |
|
This backport seem outside the scope of the kind of "critical bug fixes" I would expect in an LTS. As such, it should not be merged. |
|
This is very much in the scope of LTS bugfixes, as it solves a number of longstanding, critical issues. |
|
If you merge a "bugfix" of this scope, which could introduce entirely new bugs, I hope you are prepared to extend bugfix support for the LTS release into the "security fixes only" phase. |
|
The code introduced in the PR has been used on master for some time and has been tested pretty thoroughly because of this. At this point, we’ve caught most of the issues with the initial implementation. This is why we didn’t merge this PR before 3.12 was released, because we did expect there to be issues, but I’m pretty confident we’ve caught them at this point. |
|
Closing due to inactivity, though I'll keep the fork and branch around in case we need to revive it. The urgent bug was fixed in #18576, so the backport is less of a priority now methinks. |
This PR backports the recent changes to the Ember compilation pipeline in Ember 3.13, which dynamically compiles the Ember source based on a dependent app's browser targets.
The list of PRs to backport was supplied by @pzuraq.