Build reform #20675
Build reform #20675
Conversation
|
Notes to self on next steps:
|
|
I think "All Tests" jobs are taking forever because filtering down suites by URL isn't working, so we end up running the whole suite many times. |
packages/@ember/-internals/glimmer/tests/integration/components/curly-components-test.js
Outdated
Show resolved
Hide resolved
…anges I'm working to land a [Build Reform](emberjs/ember.js#20675) branch in ember-source that, among other things, uses only rollup for Ember's prepublication build, ensuring that it's all ES-module clean. The inter-package imports within `ember-source/dist/packages` switch from being package names (which require non-standard resolving to work) to relative imports (which do not). As a consequence of that it's simpler to ship all of `dist/packages` and `dist/dependencies` together as `dist/packages`. So our compat adapter needs to tolerate `dist/dependencies` not existing. The special handling we had for enumerating the contents of `dist/dependencies` and removing them from package.json dependencies was only needed to deal with the magical inter-package resolving, so it's correct that it becomes a no-op for these new ember versions.
…anges I'm working to land a [Build Reform](emberjs/ember.js#20675) branch in ember-source that, among other things, uses only rollup for Ember's prepublication build, ensuring that it's all ES-module clean. The inter-package imports within `ember-source/dist/packages` switch from being package names (which require non-standard resolving to work) to relative imports (which do not). As a consequence of that it's simpler to ship all of `dist/packages` and `dist/dependencies` together as `dist/packages`. So our compat adapter needs to tolerate `dist/dependencies` not existing. The special handling we had for enumerating the contents of `dist/dependencies` and removing them from package.json dependencies was only needed to deal with the magical inter-package resolving, so it's correct that it becomes a no-op for these new ember versions.
|
Confirmed in a real app's production build that this reduces vendor.js by 48kB. Most of that is probably from using decorator-transforms internally, so that we're not forced to transpile all other class features. |
|
I audited the differences caused by this PR in the AMD loader contents of a real app in both dev and prod. The only change is that |
| // ember.debug.js that publishes in the ember-source NPM package. That copy is | ||
| // essentially an optimization if you happen to be doing development under the | ||
| // default babel targets. | ||
| 'PREBUILT', |
There was a problem hiding this comment.
This concept stops being useful in this PR. It was very specific to the old optimization where sometimes ember would use a prebuilt bundle and other times it wouldn't.
| @@ -0,0 +1,202 @@ | |||
| /* eslint-disable */ | |||
|
|
|||
| // This file was derived from the output of the classic broccoli-based build of | |||
There was a problem hiding this comment.
My intent for these three amd-compat-entrypoints files is that they're capturing a final snapshot of what we ship in the backward-compatible AMD bundles. We should consider them frozen going forward. New modules shouldn't go into them.
Instead, new modules should ship like normal addon modules from treeForAddon.
The rationale here is that moving existing modules out of vendor.js is a breaking change. But new modules that have never been in vendor.js can avoid ever going there.
This PR doesn't introduce any new modules doing things the new way, but I intend to do that in my next PR.
| @@ -1,12 +0,0 @@ | |||
| 'use strict'; | |||
There was a problem hiding this comment.
Bundle the babel helpers is now handled by rollup.
This causes the one visible difference in the contents of the AMD loader after this PR: we used to expose ember-babel there, just because that's where we happened to store our helpers.
| // transform, since some tests use decorators and class fields | ||
| ['@babel/plugin-proposal-decorators', { legacy: true }], | ||
| ['@babel/plugin-proposal-class-properties', { loose: true }], | ||
| module.exports = function canaryFeatures() { |
There was a problem hiding this comment.
This is just changing from a broccoli transform to a piece of babel config.
| const isProduction = process.env.EMBER_ENV === 'production'; | ||
| let ember = new Funnel(tree, { | ||
| destDir: 'ember', | ||
| include: [`ember.${which}.js`, `ember.${which}.map`, 'ember-testing.js', 'ember-testing.map'], |
There was a problem hiding this comment.
Here I'm returning to a simpler pattern that we've used in the past, which is that the prepublication build produces both ember.debug.js and ember.prod.js. The benefit of this is that we don't need to bring the complete prepublication build machinery into every app.
This works now because we don't need @babel/preset-env to do anything in our prepublication build. We can produce complete bundles and still run them through preset-env at app-build time.
| if ( | ||
| !isProduction && | ||
| // do the running applications targets match our prebuilt assets targets? | ||
| PRE_BUILT_TARGETS.every((target) => targets.includes(target)) && |
There was a problem hiding this comment.
This optimization depends on an assumption that is no longer true. Which is that producing an ember bundle necessarily implies using some preset-env targets.
There are no targets applied to our prebuilt bundles. It's now possible and desirable to instead pass the whole bundle through preset-env at the point where the app provides the targets.
Overall this generates production output that's about 48kB smaller.
|
|
||
| moduleFor( | ||
| moduleForDevelopment( |
There was a problem hiding this comment.
Previously, these modules were stripped from production-mode builds of the test suite using broccoli shenanigans.
Now all test modules are pulled into the test suite via import.meta.glob, and we use this moduleForDevelopment utility to control whether they execute.
| var result = callback.apply(this, reified); | ||
| if (!deps.includes('exports') || result !== undefined) { | ||
| exports = result; | ||
| } |
There was a problem hiding this comment.
This is a whole extra AMD loader that ships in bundled ember that is inert in typical usage in the browser (because the AMD loader from loader.js is present instead). It apparently still exists to make the bundles evaluatble in node.
But it lacked a feature found in loader.js that we happen to want to use. So this ports that feature into it.
| @@ -0,0 +1,77 @@ | |||
| <!DOCTYPE html> | |||
There was a problem hiding this comment.
This isn't really new, it moved from tests/index.html to follow vite conventions.
…sent. Which ensures that Ember is strictly following the ES module spec. It also makes Ember's prepublication build use Rollup instead of a custom broccoli pipeline. Again, because we want the ES module graph driving the build. This step is Rollup instead of Vite because there's enough backward-compatibility weirdness that Vite's more opinionated packaging up of Rollup features was not helpful.
Co-authored-by: Katie Gengler <katie@kmg.io>
This makes Ember's test suite build with Vite, with no AMD loader present. Which ensures that Ember is strictly following the ES module spec.
It also makes Ember's prepublication build use Rollup instead of a custom broccoli pipeline. Again, because we want the ES module graph driving the build. This step is Rollup instead of Vite because there's enough backward-compatibility weirdness that Vite's more opinionated packaging up of Rollup features was not helpful.
TODOs:
ember-testing, etc)expand the smoke-tests so they exercise the non-prebuilt legacy bundle code path(this is not a thing now, we've returned to shipping ember.prod.js for the legacy bundles so we can avoid shipping our entire legacy bundle build into apps)