WIP Sort addon css by package name #1866
Conversation
raycohen
changed the title
…ct order in advance" This reverts commit 7d31be5.
| } | ||
| } else { | ||
| result.push(resolve.sync(mod, options)); | ||
| } | ||
| } | ||
| if (styles.length) { | ||
| result = [...styles, ...result]; |
There was a problem hiding this comment.
this was unshifting the group of styles from each package to the overall list, which reverses the order and we no longer want. then in implicitScriptsAsset the entire list was getting (re-)reversed. We don't need that reverse either now.
|
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 👍 |
|
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 |
Fixes #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.