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

Reverse order of styles & scripts #1055

Merged
merged 12 commits into from
Feb 21, 2025
Merged

Reverse order of styles & scripts #1055

merged 12 commits into from
Feb 21, 2025

Conversation

TheOtterlord
Copy link
Member

@TheOtterlord TheOtterlord commented Dec 21, 2024

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.

Copy link

changeset-bot bot commented Dec 21, 2024

🦋 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

Copy link
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
Member

MoustaphaDev commented Jan 3, 2025

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
Member Author

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

@TheOtterlord
Copy link
Member Author

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

@MoustaphaDev
Copy link
Member

Anything needed to get this merged?

@TheOtterlord
Copy link
Member Author

Nope. I think this can merge!

@MoustaphaDev
Copy link
Member

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
Member

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

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.

@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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