[BUGFIX lts] Compile Ember dynamically in consuming applications #18208

pzuraq · 2019-07-17T05:24:27Z

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:

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.
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.

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.

rwjblue · 2019-07-22T21:50:12Z

Looks like the CI failures are related:

not ok 1 App Boot > App boots and routes to a URL
  ---
  message: "Died on test #1     at Object.<anonymous> (/home/travis/build/emberjs/ember.js/tests/node/app-boot-test.js:8:9): Could not find module `url` imported from `(require)`"
  severity: failed
  actual: null
  expected: undefined
  stack: Error: Could not find module `url` imported from `(require)`
    at missingModule (/home/travis/build/emberjs/ember.js/dist/ember.js:247:11)
    at findModule (/home/travis/build/emberjs/ember.js/dist/ember.js:258:7)
    at requireModule (/home/travis/build/emberjs/ember.js/dist/ember.js:24:15)
    at installProtocolForURL (/home/travis/build/emberjs/ember.js/dist/ember.js:6561:17)
    at new Environment$1 (/home/travis/build/emberjs/ember.js/dist/ember.js:6601:7)
    at Function.create (/home/travis/build/emberjs/ember.js/dist/ember.js:6613:14)
    at FactoryManager.create (/home/travis/build/emberjs/ember.js/dist/ember.js:1125:33)
    at instantiateFactory (/home/travis/build/emberjs/ember.js/dist/ember.js:929:63)
    at _lookup (/home/travis/build/emberjs/ember.js/dist/ember.js:861:12)
    at processInjections (/home/travis/build/emberjs/ember.js/dist/ember.js:969:26)

runspired · 2019-07-27T09:39:02Z

Just a heads up, this PR may not be enough.

Currently in ember-source 3.11 when native extends is mixed with .extend travis-web fails against ember-data when:

targets includes "last 2 edge"
AND targets does NOT include IE11

It passes when:

targets does not include edge
AND targets does NOT include IE11
targets includes "last 1 edge"
AND targets does NOT include IE11
targets does not include "edge" or includes "last 1 edge"/"last 2 edge"
AND targets INCLUDES IE11

You can reproduce this by:

cloning ember-data
running yarn install
running node ./bin/packages-for-commit.js
running yarn test-external:travis-web --skip-smoke-test

This test will fail and you will find travis-web in the directory ../__external-test-cache/travis-web in a state wherein you may alter the targets.js file and CI status as desired and run tests as normal in an ember app.

pzuraq · 2019-07-27T14:15:08Z

Based on your description @runspired, this PR should fix those issues as well. I believe the problem is the same as #18084, where specific targets are causing us to ship the wrong build and causing the conflict. With this PR, we will be compiling Ember using the same targets as the consuming application, so this type of conflict won't be possible anymore.

BnitoBzh · 2019-07-29T09:25:14Z

Hello everyone, when do you think this problem can be solved? We need our production applications to run on ios9, now for several weeks we are having trouble. The only solution we have now is to edit the build of applications after each deployment by replacing all const with var.

pzuraq · 2019-07-30T15:25:40Z

Hey, sorry, have been working on perf tuning for tracked properties in 3.13 this last week. I'm wrapping that up, should be able to get to this sometime this week.

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.

rwjblue · 2019-08-15T20:14:36Z

ember-cli-build.js

+
+/**
+ * There isn't a way for us to override targets through ember-cli-babel, and we
+ * don't want to introduce that functionality for reasons. This is a quick and


I'd like to address this over in ember-cli-babel (in a follow up), would you mind making an issue over there about the general use case?

BnitoBzh · 2019-08-15T20:44:47Z

Hi ! is this pull request can be merged into ember 3.12.x or 3.11.x ?

pzuraq · 2019-08-15T21:22:55Z

This is marked for backport to LTS, so it will be backported to 3.12 for sure

josemarluedke · 2019-08-20T15:46:05Z

lib/index.js

+    if (!isProduction && PRE_BUILT_TARGETS.every(target => targets.includes(target))) {
+      ember = new Funnel(tree, {
+        destDir: 'ember',
+        include: ['ember.debug.js', 'ember.debug.map', 'ember-testing.js', 'ember-testing.map'],


Should ember-template-compiler.js be present here as well?

This change might have caused this: embroider-build/embroider#311.

That seems plausible, mind making a PR so we can focus discussion over there?

That's done at #18291.

Turbo87 · 2019-08-26T12:25:24Z

why is this PR marked as "BUGFIX lts"? the description is not mentioning any bugs that are being fixed by this and it seems like a pretty significant change for those that expect LTS to work the same way as before 🤔

rwjblue · 2019-08-26T14:04:23Z

@Turbo87 - There is a whole category of issues that this PR will ultimately resolve. A few examples:

These changes are currently only in beta (3.13.0-beta.3 I think?), and will only be backported to LTS (likely 3.12 if we do it, not 3.8) if we can be extremely certain that they will not cause more issues than it solves.

snewcomer · 2019-10-18T15:54:34Z

Has this been backported to 3.12? Is this something I can help with. This is the internal addon we have to fix this temporarily (we are on 3.12)

'use strict';
const replace = require('broccoli-replace');

module.exports = {
    name: 'temporary-ember-fix',
    postprocessTree(type, tree) {
        if (type !== 'all') {
            return tree;
        }

        return replace(tree, {
            files: ['**/vendor*.js'],
            patterns: [{
                match: /const\s/g,
                replacement: 'var '
            }]
        });
    },

    isDevelopingAddon() {
        return true;
    }
};

pzuraq · 2019-10-18T16:03:00Z

I don't believe it has yet, but it definitely should be. It's a large change, but it's been pretty stable so far, no more issues, and it shouldn't be too hard to pull it onto 3.12 (originally we were hoping to get it in then, but we decided against it since we didn't have enough time for testing).

If you can take a crack at it, that'd be very helpful, otherwise I'll try to find some time in the near future to get it going.

rwjblue · 2019-10-18T16:57:39Z

Ya, the major thing to do is identify all the bugfix commits for these files that happened after this initially landed so we have a clearer idea of what to backport.

ssnielsen · 2019-11-01T10:00:36Z

Very glad to see that this has/is being worked on :) We've recently upgraded to 3.12.0 and have had reports from our iOS 9 users about the issue mentioned in #18084

@rwjblue What do you estimate is the timeframe wrt. getting this fix into a future 3.12.x release?

rwjblue · 2019-11-01T13:24:01Z

TBH, I don't know. We really need to figure out the full list of changes needed to backport, that is probably the main blocker. If someone has time to dig through the commit history for anything build related since this PR landed and make a PR targetting the lts-3-12 branch that would be super helpful and make it massively more likely for it to happen "soon". After that actually doing the release is pretty simple once we get the changes there.

pzuraq · 2019-11-01T15:43:08Z

FWIW I made https://github.com/pzuraq/ember-class-constructor-fix which should fix most of the general issues in older builds. It's a more general fix too, which means it'll work prior to 3.12 as well.

#18084 is particularly strange though, since it appears that the legacy build is not being used, despite very clear legacy targets. Will comment over there to try to get more details.

snewcomer · 2019-11-01T17:58:42Z

This is phenomenal! I love the approach. Is this the fix or is backporting changes the fix? Seems like the former? (if the latter, I can help today and this weekend)

pzuraq · 2019-11-04T17:53:30Z

I think backporting would still be useful, if possible, but I don't think we need to spend extra cycles on it. This fix should work in any cases where we eagerly opt into the legacy build even if we shouldn't actually do that, and as I was working through the fix I thought that was the only possible thing that could accidentally happen.

This is why #18084 is so strange to me, based on the code I don't think it should be possible. We basically only opt-into legacy IFF classes need to be compiled OR another feature needs to be compiled, so

If classes are compiled, then the legacy build is used, and user's classes are compiled, so nothing to fix.
If classes are not compiled, and nothing else triggers the legacy build, then there's nothing to fix.
If classes are not compiled, but something else is and triggers the legacy build, then user code is not compiled, but framework code is, and the fix patches it.

I don't think it should be possible for user classes to be compiled, and for the framework to not be the legacy build, but maybe there's something I wasn't considering.

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

pzuraq mentioned this pull request Jul 17, 2019

Version 3.10.1 regression: use of const in strict mode, causing critical errors in e.g. Safari 9 #18084

Closed

pzuraq force-pushed the bugfix/compile-ember-dynamically branch 17 times, most recently from ae86d5c to f4229fb Compare July 19, 2019 20:27

pzuraq force-pushed the bugfix/compile-ember-dynamically branch 6 times, most recently from 810550e to de69708 Compare August 2, 2019 17:26

pzuraq force-pushed the bugfix/compile-ember-dynamically branch from de69708 to 2ac757a Compare August 9, 2019 21:03

pzuraq force-pushed the bugfix/compile-ember-dynamically branch 7 times, most recently from b4de6b7 to e1c0e0c Compare August 13, 2019 15:55

Chris Garrett added 2 commits August 14, 2019 11:33

[BUGFIX lts] Updates based on feedback

e526a04

pzuraq force-pushed the bugfix/compile-ember-dynamically branch from e1c0e0c to e526a04 Compare August 14, 2019 18:34

rwjblue reviewed Aug 15, 2019

View reviewed changes

rwjblue merged commit af48faa into master Aug 15, 2019

rwjblue deleted the bugfix/compile-ember-dynamically branch August 15, 2019 20:19

josemarluedke mentioned this pull request Aug 20, 2019

Cannot find module 'ember-source/vendor/ember/ember-template-compiler' with Ember 3.13.0-beta.3 embroider-build/embroider#311

Closed

josemarluedke reviewed Aug 20, 2019

View reviewed changes

rwjblue mentioned this pull request Aug 20, 2019

Could not find module @babel/runtime/helpers/esm/createClass #18290

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUGFIX lts] Compile Ember dynamically in consuming applications #18208

[BUGFIX lts] Compile Ember dynamically in consuming applications #18208

pzuraq commented Jul 17, 2019

rwjblue commented Jul 22, 2019

runspired commented Jul 27, 2019

pzuraq commented Jul 27, 2019

BnitoBzh commented Jul 29, 2019

pzuraq commented Jul 30, 2019

rwjblue Aug 15, 2019

BnitoBzh commented Aug 15, 2019

pzuraq commented Aug 15, 2019

josemarluedke Aug 20, 2019

rwjblue Aug 20, 2019

josemarluedke Aug 20, 2019

Turbo87 commented Aug 26, 2019

rwjblue commented Aug 26, 2019

snewcomer commented Oct 18, 2019

pzuraq commented Oct 18, 2019

rwjblue commented Oct 18, 2019

ssnielsen commented Nov 1, 2019

rwjblue commented Nov 1, 2019

pzuraq commented Nov 1, 2019

snewcomer commented Nov 1, 2019 •

edited

Loading

pzuraq commented Nov 4, 2019 •

edited

Loading

[BUGFIX lts] Compile Ember dynamically in consuming applications #18208

[BUGFIX lts] Compile Ember dynamically in consuming applications #18208

Conversation

pzuraq commented Jul 17, 2019

rwjblue commented Jul 22, 2019

runspired commented Jul 27, 2019

pzuraq commented Jul 27, 2019

BnitoBzh commented Jul 29, 2019

pzuraq commented Jul 30, 2019

rwjblue Aug 15, 2019

Choose a reason for hiding this comment

BnitoBzh commented Aug 15, 2019

pzuraq commented Aug 15, 2019

josemarluedke Aug 20, 2019

Choose a reason for hiding this comment

rwjblue Aug 20, 2019

Choose a reason for hiding this comment

josemarluedke Aug 20, 2019

Choose a reason for hiding this comment

Turbo87 commented Aug 26, 2019

rwjblue commented Aug 26, 2019

snewcomer commented Oct 18, 2019

pzuraq commented Oct 18, 2019

rwjblue commented Oct 18, 2019

ssnielsen commented Nov 1, 2019

rwjblue commented Nov 1, 2019

pzuraq commented Nov 1, 2019

snewcomer commented Nov 1, 2019 • edited Loading

pzuraq commented Nov 4, 2019 • edited Loading

snewcomer commented Nov 1, 2019 •

edited

Loading

pzuraq commented Nov 4, 2019 •

edited

Loading