search

embroider-build / embroider

Compiling Ember apps into spec-compliant, modern Javascript.
MIT License
329 stars 137 forks source link

WIP Sort addon css by package name #1866

Open raycohen opened 1 month ago

raycohen commented 1 month ago

Fixes https://github.com/embroider-build/embroider/issues/1862

While broccoli-concat doesn't guarantee order, the effective order of addon css in (most?) classic builds seems to be alphabetical by package name.

Also, the order of app.import'd styles was being reversed from what the classic build had, in addition to not coming before addon css.

raycohen commented 1 month ago

stable and main differ slightly inside impliedAddonAssets

BlueCutOfficial commented 1 month ago

impliedAddonAssets code is reused to replace the assets generated by compat-app-builder.ts with virtual contents. If this PR is merged we probably want to double-check #1805 and #1811 and reintegrate the changes in the new virtual-content generators.

mansona commented 1 month ago

Just a quick note on how to make sure this continues to work for the upcoming major release: @ef4 is correct that you should target stable with a change like this so that it could be released sooner than the next major, but we can't ensure that this is going to keep working unless we make sure to add a test that verifies the output is in the correct order. If this gets merged to stable and we then forward-port the test to the main branch we can ensure that the sort ordering is still correct even though the implementation is completely different 👍

raycohen commented 1 month ago

I will look into testing this. Right now we're getting by with patch-package (with a slightly different diff) but we'll want to get off that eventually