Emit our packages in treeforAddon and treeForAddonTestSupport

[ef4]

This is a step toward making ember-source a less weird and badly-behaved addon. By emitting its code in treeForAddon instead of treeForVendor, ember-source actually exposes its ES modules to the rest of the system and can benefit from ember-auto-import or embroider.

So far this draft PR is enough to make an example application boot and pass tests. This repo's tests won't be passing yet because they're hard-coded to the old way the assets were generated.

Next I'd like to switch the prepublication build to use just rollup, without broccoli & ember-cli, because it would be easier to produce a conventional addon structure and easier to reimplement the test suite as modules rather than scripts.

[ef4]

One actual semantic change I made here is that there was some probably-unintentional duplication between the default and test bundles. In an app this would manifest as having code duplicated between vendor.js and test-support.js, with identical modules overwriting each other in the runtime loader.

I removed ember-testing from vendor.js in dev builds (it was already removed from prod builds). It's present in test-support.js, so was never necessary in vendor.js

I also removed @ember/debug from test-support.js because this package is always present in vendor.js anyway (even in production, where its functions have empty implementations but still exist).

[ef4]

Another change here is that the bundled ember was wrapping loader.js in an extra layer of loader API that seems unnecessary.

[ef4]

@wycats @gitKrystan this is relevant to our discussion last week.

[gitKrystan]

I've been trying to dogfood this in some or our apps. In our ~1yo app (running embroider), I am running into:

Build Error (WaitForTrees)

Cannot find module 'ember-source/vendor/ember/ember-template-compiler' from '/private/var/folders/xx/2vtrs0m95q36gnbsvy_2293c0000gn/T/embroider/24c694'


Stack Trace and Error Report: /var/folders/xx/2vtrs0m95q36gnbsvy_2293c0000gn/T/error.dump.ac984c420315c3f4d362f090ce7682dd.log

It turns out that this line in embroider/compat-app looks for ember-source/vendor/ember/ember-template-compiler (note that the path has '/ember/' in it): https://github.com/embroider-build/embroider/blob/01575a4d3a86b8b329c75024aa77b2539a6dad5d/packages/compat/src/compat-app.ts#L309

But this PR puts it in ember-source/vendor/ember-template-compiler (note that the path does not have '/ember/' in it): https://github.com/emberjs/ember.js/pull/20349/files#diff-e835654135548f75db36136807171b4d12d3514e043f4f80a146b88cc668fba4R233

If I change the ember-cli-build concatBundle config to:

outputFile: 'vendor/ember/ember-template-compiler.js',

then I get this error during build:

Cannot find module '/Users/krystanhuffmenne/Code/tilde/okapi/okapi-gui/node_modules/ember-source/dist/vendor/ember-template-compiler.js'
Require stack:
- /Users/krystanhuffmenne/Code/tilde/okapi/okapi-gui/node_modules/ember-cli-htmlbars/lib/utils.js
- /Users/krystanhuffmenne/Code/tilde/okapi/okapi-gui/node_modules/ember-cli-htmlbars/lib/ember-addon-main.js
- /Users/krystanhuffmenne/Code/tilde/okapi/okapi-gui/node_modules/ember-cli/lib/models/package-info-cache/package-info.js
- /Users/krystanhuffmenne/Code/tilde/okapi/okapi-gui/node_modules/ember-cli/lib/models/package-info-cache/index.js
- /Users/krystanhuffmenne/Code/tilde/okapi/okapi-gui/node_modules/ember-cli/lib/models/project.js
- /Users/krystanhuffmenne/Code/tilde/okapi/okapi-gui/node_modules/ember-cli/lib/utilities/get-config.js
- /Users/krystanhuffmenne/Code/tilde/okapi/okapi-gui/node_modules/ember-cli/lib/utilities/instrumentation.js
- /Users/krystanhuffmenne/Code/tilde/okapi/okapi-gui/node_modules/ember-cli/lib/cli/index.js
- /Users/krystanhuffmenne/Code/tilde/okapi/okapi-gui/node_modules/ember-cli/bin/ember

But if I change the above-mentioned line in embroider/compat-app to ember-source/vendor/ember-template-compiler (without '/ember/') then the app builds properly and everything looks fine in dev.

In fact, I am having the same issue with a test app created with ember new test-app --embroider --yarn.

[gitKrystan]

Everything appears to work on a fresh app created with ember new test-app --yarn (no embroider).

But in our non-embroider app running ember-cli 4.11, I get no build errors, but a runtime error:

vendor.js:7612 Uncaught ReferenceError: Ember is not defined
    at vendor.js:7612:119
    at vendor.js:7681:3

vendor.js:266 Uncaught Error: Could not find module `@ember/application` imported from `direwolf/app`
    at missingModule (vendor.js:266:11)
    at findModule (vendor.js:277:7)
    at Module.findDeps (vendor.js:187:24)
    at findModule (vendor.js:281:11)
    at requireModule (vendor.js:43:15)
    at direwolf.js:55708:13

I'm having more trouble debugging this one and would love any suggestions.

[wycats]

@ef4 @gitKrystan I wonder if we should take the opportunity to express these locations in an exports field of ember-source, rather than updating downstream libraries to be aware of the new location.

(It might have other problematic implications, though, since some tools treat the presence of an export field as a broad-base dopt-in for all kinds of "modern" features).

[gitKrystan]

During dog-fooding non-embroider apps with @wycats, we're noticing that multiple addons (e.g. ember-cli-deprecation-workflow) expect require.has('@ember/...') === true, but after this PR, it is false. We're thinking this is expected behavior after the change, but it seems like a pretty significant breaking change.

To reproduce this issue, add ember-cli-deprecation-workflow to a fresh non-embroider ember app and use this version of ember-source. The app will build, but in runtime, you will see Uncaught ReferenceError: Ember is not defined

Additionally, we attempted to backport this change to Ember 3.28 and found that somehow it loses the Ember global.

We are continuing to investigate these issues as well as doing more dogfooding in Embroider apps, but we'd welcome any feedback if you have ideas.

[ef4]

We're thinking this is expected behavior after the change, but it seems like a pretty significant breaking change.

This is not expected, nothing in the PR should have intentionally affected that. Maybe the double-loader shenanigans I removed was facilitating that behavior.

But in general, in a classic build, everything emitted by treeForAddon will definitely still be require.has.

[ef4]

I investigated this further.

we're noticing that multiple addons (e.g. ember-cli-deprecation-workflow) expect require.has('@ember/...') === true, but after this PR, it is false.

That isn't quite the whole story. This PR doesn't change what require.has() returns, as long you ask from an ES module.

ember-cli-deprecation-workflow is injecting a script, not a module. Accessing ember's modules from a script is an ill-defined thing to do. It's timing-dependent. In the current implementation, it happens to work because of the relative timing of where ember's stuff and ember-cli-deprecation-workflow's stuff end up in vendor.js. Nothing is actually guaranteeing that timing, and the order happens to change with this PR.

[ef4]

This class of problem is why I have been saying that deprecating the AMD loader is one of our high priorities.

[kategengler]

Is this still relevant? It is a draft PR