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.

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

Module resolver: virtualize vendor.css #1805

Merged
merged 4 commits into from
Apr 10, 2024
Merged

Module resolver: virtualize vendor.css #1805

merged 4 commits into from
Apr 10, 2024

Conversation

BlueCutOfficial
Copy link
Collaborator

@BlueCutOfficial BlueCutOfficial commented Feb 15, 2024

This PR replaces the assets/vendor.css file generated in the rewritten-app with an entrypoint returning a virtual CSS:

- <link integrity="" rel="stylesheet" href="/assets/vendor.css">
+ <link integrity="" rel="stylesheet" href="/@embroider/core/vendor.css">

The content that was previously written in assets/vendor.css is now generated by the virtual-vendor-styles's functions.

How to test

On the main branch, to get an /assets/vendor.css endpoint in the rewritten app, the vite-app needs to include at least one V1 addon that exposes CSS. On the present virtual-vendor-style branch, the entry point is always there, but the content will be empty if no V1 addon exposes CSS.

The POC branch BlueCutOfficial:poc-virtual-vendor-style eases testing: it installs two classic addons providing CSS styles. You can also install them this way:

pnpm add --save-dev ember-tag-input ember-simple-tree

Once the vite-app has these dependencies, you can test what happens when there is a vendor.css.

  • On the poc-virtual-vendor-style branch:
    • Start the vite-app
    • Visit http://127.0.0.1:4200
      • 👀 You should see the correctly styled components coming from ember-tag-input and ember-simple-tree
      • 👀 In the network tab, vendor.css should be returned as a CSS file containing the addons styles
    • Build the vite-app
      • 👀 In dist/assets, there should be a fingerprinted main-*.css containing the addons styles
      • 👀 The stylesheet pointing to this file should be present in dist/index.html

@BlueCutOfficial BlueCutOfficial changed the title Virtual vendor styles entry point Module resolver: virtualize vendor.css Feb 19, 2024
@mansona
Copy link
Member

mansona commented Feb 26, 2024

With a bit of experimentation on a vanilla Vite app @BlueCutOfficial discovered that we can probably get away with properly virtualising a raw css file making use of ?direct when internally redirected.

We've been struggling to make sure that the content-type is correct in an embroider test, there are a few things to try out but we could also try a "post middleware" to edit the content type of css responses. https://vitejs.dev/guide/api-plugin.html#configureserver

if adding a post middleware to edit the content type doesn't work then we could just implement a normal middleware to handle the virtual css response correctly. Then for build mode we would just use the emit file as we wanted to but couldn't because it doesn't work in dev mode.

@mansona mansona force-pushed the virtual-entry-point branch 7 times, most recently from 425215f to 0c31ba0 Compare February 29, 2024 17:43
@mansona mansona force-pushed the virtual-entry-point branch 6 times, most recently from 44981c2 to 00e95a1 Compare March 12, 2024 14:38
@mansona mansona force-pushed the virtual-entry-point branch 3 times, most recently from 9d46807 to 90a8cad Compare April 3, 2024 11:57
@BlueCutOfficial BlueCutOfficial changed the base branch from virtual-entry-point to main April 3, 2024 12:06
Comment on lines +62 to +64
expectFile('./index.html').matches(
'data-original-filename="vendor.css">',
'does not have data-original-filename vendor.css'
'has data-original-filename vendor.css'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The compat-app-builder used to compute a boolean implicitStyles and add the stylesheet only if true. With this PR, we simplify this and add the stylesheet without condition.

@BlueCutOfficial BlueCutOfficial marked this pull request as ready for review April 3, 2024 16:10
Copy link
Member

@mansona mansona left a comment

Choose a reason for hiding this comment

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

LGTM 🎉 💪

@mansona mansona added the enhancement New feature or request label Apr 10, 2024
@mansona mansona merged commit cb8efec into embroider-build:main Apr 10, 2024
92 checks passed
@github-actions github-actions bot mentioned this pull request Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants