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

Fastboot lazy css support #1140

Merged
merged 11 commits into from Mar 4, 2022
Merged

Fastboot lazy css support #1140

merged 11 commits into from Mar 4, 2022

Conversation

ef4
Copy link
Contributor

@ef4 ef4 commented Feb 24, 2022

This fixes a crash that can occur if an app tries to lazily load CSS in fastboot.

This change works by making all the lazy CSS eager. This is strictly better than being broken. Further work can follow to make the lazy chunks actually lazy in fastboot, such that we detect during rendering which ones were wanted and only insert link tags for those ones.

Apps that don't use fastboot should be unaffected.

So that we can use process.env.EMBROIDER_TEST_SETUP_OPTIONS to configure the apps.
This achieves correctness by avoiding use of the MiniCssExtractPlugin runtime inside fastboot.

Right now It works by making all the lazy CSS eager. This is strictly better than being broken. Further work can follow to make the lazy chunks actually lazy in fastboot, such that we detect during rendering which ones were wanted and only insert link tags for those ones.
@ef4 ef4 marked this pull request as ready for review March 4, 2022 04:57
This was here to try to make each filter match an exact module name, but that apparently causes their child modules not to run.
@ef4
Copy link
Contributor Author

ef4 commented Mar 4, 2022

This is working as intended, but the test failures are places where we already had tests for lazy CSS in engines that have now been forced to be eager.

Next step it to investigate how those were ever working -- it's possible they weren't and the tests simply didn't hit the bad cases.

They only work without fastboot.

They never really worked with fastboot, but we were not running them properly in CI.
@ef4
Copy link
Contributor Author

ef4 commented Mar 4, 2022

I confirmed that the lazy engines CSS case was actually broken all along in fastboot, and the test wasn't running properly in CI before.

So I'm adjusting the tests to guard that case and only assert that lazy engine CSS is lazy in non-fastboot apps.

@ef4 ef4 merged commit 13273f4 into main Mar 4, 2022
@ef4 ef4 deleted the fastboot-lazy-css branch March 4, 2022 07:03
@ef4 ef4 added the bug Something isn't working label Mar 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant