Move responsibility of booting the app from Embroider internals to the Ember app #1957
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BlueCutOfficial
force-pushed
the
new-app-boot
branch
2 times, most recently
from
0a017b4 to
b063489
Compare
BlueCutOfficial
force-pushed
the
new-app-boot
branch
4 times, most recently
from
b3d26ec to
ac6a130
Compare
|
I think this PR should remove the support for |
BlueCutOfficial
force-pushed
the
new-app-boot
branch
3 times, most recently
from
1452428 to
bf49153
Compare
BlueCutOfficial
changed the title
BlueCutOfficial
force-pushed
the
new-app-boot
branch
11 times, most recently
from
ae5593a to
54b0cef
Compare
…er/ember-app-boot.js to the new app-boot.js file
BlueCutOfficial
force-pushed
the
new-app-boot
branch
3 times, most recently
from
8236571 to
6f393a8
Compare
BlueCutOfficial
force-pushed
the
new-app-boot
branch
from
6f393a8 to
0efc667
Compare
BlueCutOfficial
force-pushed
the
new-app-boot
branch
from
0efc667 to
6f2af28
Compare
mansona
reviewed
Jun 20, 2024
mansona
reviewed
Jun 20, 2024
mansona
reviewed
Jun 20, 2024
BlueCutOfficial
force-pushed
the
new-app-boot
branch
3 times, most recently
from
88ffed3 to
3ea4714
Compare
BlueCutOfficial
commented
Jun 20, 2024
Comment on lines
698
to
592
| expectAudit | ||
| .module('./node_modules/my-addon/_app_/components/hello-world.js') | ||
| .resolves('my-addon/components/hello-world') | ||
| .to('./node_modules/my-addon/components/hello-world.js', 'remapped to precise copy of my-addon'); |
There was a problem hiding this comment.
I removed this assertion because I need to go through this resolve path anyway to test the content of my-addon/components/hello-world.js in another test. So I simply moved the message "remapped to precise copy of my-addon" to a comment in that other test.
BlueCutOfficial
force-pushed
the
new-app-boot
branch
from
3ea4714 to
aa95998
Compare
BlueCutOfficial
commented
Jun 21, 2024
Comment on lines
-560
to
-576
| window.define("my-app/components/second-choice", function () { | ||
| return importSync("#embroider_compat/components/second-choice"); | ||
| }); | ||
| window.define("my-addon/synthetic-import-1", function () { | ||
| return importSync("../synthetic-import-1"); | ||
| }); | ||
| import Component from '@ember/component'; | ||
| import layout from '../templates/components/hello-world'; | ||
| import computed from '@ember/object/computed'; | ||
| import somethingExternal from 'not-a-resolvable-package'; | ||
| import { importSync } from "@embroider/macros"; | ||
| export default Component.extend({ | ||
| dynamicComponentName: computed('useDynamic', function () { | ||
| return this.useDynamic || 'default-dynamic'; | ||
| }), | ||
| layout | ||
| }); |
There was a problem hiding this comment.
Do we want to keep asserting all the imports are present, or do we just want enough proof that this is the right file that has been loaded?
BlueCutOfficial
force-pushed
the
new-app-boot
branch
from
aa95998 to
9993260
Compare
ef4
approved these changes
Jun 25, 2024
BlueCutOfficial
added a commit
that referenced
this pull request
Jul 5, 2024
BlueCutOfficial
added a commit
that referenced
this pull request
Jul 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The app-boot script is now part of the Ember app
This PR implements the 1st part of #1960:
virtual-entrypoint.tsso the app-boot no longer happens in Embroider internals.index.htmlandapp/app.jsfiles).How it drives the decision to stop the support of content-for "app-boot"
The app-boot script specifies instructions to boot the app. During the classic build, some Broccoli plugins and Ember libraries work together to generate the correct app-boot instructions, so it used to be something internal.
If you are an addon author and the way your addon works requires customizing the app-boot, you couldn't provide any configuration instructions to your users because they have no control over the app-boot anyway. But classic addons let you provide the required content yourself using the
contentForhook. There is an example in ember-cli-fastboot.With this PR, app developers now have control over the app-boot script: it's written directly in the
index.html. Classic addons won't be able to impact this script, it will be the responsibility of the app.A new error message has been implemented so if the app uses at least one classic addon providing custom content for app-boot, an explanation about how to move the instructions on the app side will be displayed.