Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes app import paths in index.html #58

Merged
merged 1 commit into from
Sep 2, 2024
Merged

Fixes app import paths in index.html #58

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions files-override/app/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@

<script src="/@embroider/core/vendor.js"></script>
<script type="module">
import Application from './app';
import environment from './config/environment';
import Application from './app/app';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because index.html is in the app folder already, the meaning of this import means that app.js is actually located at <repo>/app/app/app.js -- which is wrong.

I think the change in this PR is papering over a side-effect of the rewritten app still existing.

When observing the output, your change does make sense:
image

However, in source, it does not make sense:
image

so the index.html would need to move to the root of files-override to to have the imports make sense / not be wrong in source.

With the upcoming removal of app rewriting, the proposed change here will make sense if the index.html were moved outside of the app directory.

However (however), the change here also matches what is in the app-template base-app in the embroider repo.

We can move the index.html out of app later.

import environment from './app/config/environment';

Application.create(environment.APP);
</script>
Expand Down