Skip to content

Reverse order of styles & scripts#1055

Merged
ematipico merged 12 commits intomainfrom
output-order
Feb 21, 2025
Merged

Reverse order of styles & scripts#1055
ematipico merged 12 commits intomainfrom
output-order

Conversation

@TheOtterlord
Copy link
Copy Markdown
Member

@TheOtterlord TheOtterlord commented Dec 21, 2024 •
edited
Loading

Changes

For example, with component like the follow:

<style>
  body {
    background-color: red;
  }
</style>
<style>
  body {
    background-color: yellow;
  }
</style>

The current compiler output would render red last, even though yellow was the last style-tag defined.

<style>
  body {
    background-color: yellow;
  }
  body {
    background-color: red;
  }
</style>

Enabling the experimentalScriptOrder flag will respect the order style & script tags are defined.

<style>
  body {
    background-color: red;
  }
  body {
    background-color: yellow;
  }
</style>

Testing

  • Updated existing styles test
  • Added test for scripts order

Docs

  • Will require a section for the new experimental flag once implemented in withastro/astro.

@TheOtterlord @louisescher
Reverse order of styles & scripts
60e6d17 
Co-authored-by: Louis Escher <louisescher@proton.me>
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Dec 21, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: 188fe2e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@astrojs/compiler Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

TheOtterlord and others added 2 commits December 21, 2024 22:04
@TheOtterlord @louisescher
Fix test snapshots
c6b2287 
Co-authored-by: Louis Escher <louisescher@proton.me>
@TheOtterlord
Add changeset
68e69b1
MoustaphaDev
MoustaphaDev reviewed Jan 2, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@MoustaphaDev MoustaphaDev left a comment

Choose a reason for hiding this comment

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

Changes look good to me, I just have a suggestion on a test

@MoustaphaDev
Copy link
Copy Markdown
Member

MoustaphaDev commented Jan 3, 2025 •
edited
Loading

Not sure why the go lint job is failing, but for the js one, running pnpm check:writeshould be able to fix it.

@TheOtterlord
Copy link
Copy Markdown
Member Author

TheOtterlord commented Jan 3, 2025

It's also failing on main, so probably worth a separate PR

@TheOtterlord
Copy link
Copy Markdown
Member Author

TheOtterlord commented Jan 3, 2025

ohh, only lint fails on main. I'll fix the js one now

bluwy
bluwy approved these changes Jan 10, 2025
View reviewed changes
@TheOtterlord @bluwy
Apply suggestions from code review
c30e5fd 
Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
@MoustaphaDev
Copy link
Copy Markdown
Member

MoustaphaDev commented Jan 30, 2025

Anything needed to get this merged?

@TheOtterlord
Copy link
Copy Markdown
Member Author

TheOtterlord commented Jan 30, 2025

Nope. I think this can merge!

@MoustaphaDev
Copy link
Copy Markdown
Member

MoustaphaDev commented Feb 4, 2025

Just realized applying this as a patch might break users' sites that rely on the current behavior. Should we maybe treat this as a major change or wait for the next major version? 🤔

@ematipico
Copy link
Copy Markdown
Member

ematipico commented Feb 6, 2025

I would also argue that the changeset is very cryptic and doesn't explain the fix well. What was the bug? How are styles and scripts ordered after the fix?

@denisavitski
Copy link
Copy Markdown

denisavitski commented Feb 18, 2025

is there a chance that this will be merged soon? the issue with styles arises not only when there are two <style> tags in a component but also when you use the component in other components, and these components, in turn, are used on different pages. it seems to me that this problem has been around for a very long time, and I constantly have to check after the build to ensure everything is displayed as intended and adjust the specificity of the selectors.

MoustaphaDev
MoustaphaDev reviewed Feb 19, 2025
View reviewed changes
TheOtterlord and others added 2 commits February 19, 2025 18:25
@TheOtterlord
Use experimentalScriptOrder
8f8f469
@TheOtterlord @MoustaphaDev
Update .changeset/olive-beds-happen.md
188fe2e 
Co-authored-by: Happydev <81974850+MoustaphaDev@users.noreply.github.com>
@ematipico ematipico merged commit 0399d55 into main Feb 21, 2025
4 of 5 checks passed
@ematipico ematipico deleted the output-order branch February 21, 2025 09:49
@ematipico ematipico mentioned this pull request Mar 12, 2025
Merged
@jurajkapsz jurajkapsz mentioned this pull request Mar 26, 2025
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ematipico ematipico ematipico approved these changes

@MoustaphaDev MoustaphaDev MoustaphaDev approved these changes

+1 more reviewer

@bluwy bluwy bluwy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Astro reverses the order of <style> tags in a page/component

5 participants

@TheOtterlord @MoustaphaDev @ematipico @denisavitski @bluwy