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

>=2.3.0 something changed in scss processing #7508

Closed
1 task
marceloverdijk opened this issue Jun 28, 2023 · 18 comments · Fixed by #8592
Closed
1 task

>=2.3.0 something changed in scss processing #7508

marceloverdijk opened this issue Jun 28, 2023 · 18 comments · Fixed by #8592
Assignees
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) feat: styling Related to styles (scope)

Comments

@marceloverdijk
Copy link

marceloverdijk commented Jun 28, 2023

What version of astro are you using?

>=2.3.0

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

npm

What operating system are you using?

Mac

What browser are you using?

Chrome

Describe the Bug

I'm using official getbootstrap.com themes in a Astro side by using the scss provided by the theme.

That always worked except when upgrading from 2.2.3 to 2.3.0 after it was released.
II notices some strange css behaviour; back then in the nouislider component.

Anyway I thought it was something minor and decided to wait for an upgrade.

I now switched to another getbootstrap.com theme (using `2.2.3) and the theme works as expected.

Decided to update Astro to latest 2.7.1 and I see some unexpected styling issues still unfortunately.

E.g. this how it's rendered with 2.7.1

image

Note the position of the 2nd dropdown, and also the height is different.

What's the expected result?

But with 2.2.3 it is rendered as expected:

image

At the moment I'm unable to provide a reproducible example but if needed I will try to provide something.

I'm wondering if there changed something from 2.3.0 in terms of css/scss processing

My astro.config.mjs looks like:

import { defineConfig } from 'astro/config'

// https://astro.build/config
export default defineConfig({
  site: process.env.NODE_ENV === 'production' ? 'https://www.example.com/' : 'http://localhost:3000/',
  vite: {
    css: {
      preprocessorOptions: {
        scss: {
          quietDeps: true,
        },
      },
    },
  },
})

and postcss.config.js:

module.exports = {
  plugins: [require('autoprefixer')],
}

Link to Minimal Reproducible Example

...

Participation

  • I am willing to submit a pull request for this issue.
@marceloverdijk
Copy link
Author

Note, despite the Vite sass quiteDeps: true option I still see these warnings:

Deprecation Warning: Using / for division outside of calc() is deprecated and will be removed in Dart Sass 2.0.0.

But I see these warnings for both 2.2.3 and 2.7.1..

@bluwy
Copy link
Member

bluwy commented Jun 28, 2023

The biggest styling changes in 2.3.0 is #6816, but I'm not sure how that break things. Maybe a path is incorrectly remapped now? It would be hard for us to debug without a repro.

@bluwy bluwy added the needs repro Issue needs a reproduction label Jun 28, 2023
@marceloverdijk
Copy link
Author

Thx @bluwy , I will try to create an isolated example repo over the weekend.

@marceloverdijk
Copy link
Author

@bluwy I've created an isolated example repo here: https://github.com/marceloverdijk/astro-7508
It uses Astro 2.2.3 which works for me, but any newer version (including latest 2.8.0) seems to give some scss processing issue.
If there is anything else I can provide or help with let me know - I'm hoping to upgrade to latest Astro.

@bluwy bluwy removed the needs repro Issue needs a reproduction label Jul 10, 2023
@bluwy
Copy link
Member

bluwy commented Jul 10, 2023

Thanks! I'm able to reproduce it. Reverting #6816 entirely seems to fix it, but I still don't quite understand how that's triggered. Will try to debug this more.

@bluwy bluwy self-assigned this Jul 10, 2023
@bluwy bluwy added feat: styling Related to styles (scope) - P3: minor bug An edge case that only affects very specific usage (priority) labels Jul 10, 2023
@marceloverdijk
Copy link
Author

Thanks for the feedback @bluwy !

@marceloverdijk
Copy link
Author

@bluwy any chance changes in upcoming Astro 3.0 release will solve this. I'm currently stuck to old version without possibility to update unfortunately.

@bluwy
Copy link
Member

bluwy commented Aug 25, 2023

Hey @marceloverdijk. Unfortunately I was heads down getting some 3.0 tasks out, so I haven't get a deeper look into this, and I'll be taking some time off soon.

So the bug might still exist in 3.0, and if so I'll return to this right away after my break! (Or if someone else takes a look at it)

@marceloverdijk
Copy link
Author

OK I will try to upgrade to 3.0 anyway and give it a shot.
I will report here the findings.

@marceloverdijk
Copy link
Author

@bluwy FYI: I've updated the example repo to the latest Astro 3.0.1 but the issue is unfortunately still there.

@marceloverdijk
Copy link
Author

Thx @bluwy awesome! this is resolved. Looking forward to the next release so I can upgrade to the latest.

@marceloverdijk
Copy link
Author

I can confirm this works with latest 3.1.2 👏 🥳

@bluwy
Copy link
Member

bluwy commented Sep 22, 2023

Awesome! Thanks for the update.

@marceloverdijk
Copy link
Author

Hi @bluwy despite the original issue fixed, I see some other issues now.

With 2.3.1 a page is generated like:

image

but with 3.1.2:

image

See the spacing between the badges in the top left of the cards.

This is producible with the example repo mentioned earlier.

And in the actual app I'm working with I see now more of these small differences.
Some of I can reproduce in the example repo, but some not.
As far as I could investigate they all seem to be related to css ordering in combination with css variables.

I'm now wondering if I'm doing something wrong in my project setup...

@bluwy
Copy link
Member

bluwy commented Sep 22, 2023

I can see there's duplicated CSS in 3.0 for some reason, but the styles are still the same and I can't tell what is causing the missing space. In v2 it seems to be coming from the inline block natural spacing, but it's collapsed in 3.0 for some reason.

@marceloverdijk
Copy link
Author

Yes I also noticed (partially) duplicated css.

E.g. I have a span like:

<span class="badge bg-primary bg-opacity-10 text-primary">Hotel</span>

and the generated css contains:

L28048:

.bg-primary {
  --bs-bg-opacity: 1;
  background-color: rgba(var(--bs-primary-rgb), var(--bs-bg-opacity)) !important;
}

L28116:

.bg-opacity-10 {
  --bs-bg-opacity: 0.1;
}

L34947:

.bg-primary {
  --bs-bg-opacity: 1;
  background-color: rgba(var(--bs-primary-rgb), var(--bs-bg-opacity)) !important;
}

The .bg-primary is indeed duplicated, but the bg-opacity-10 not.
The problem now is that the --bs-bg-opacity is overwritten again.

So the (partially) duplicates css is causing an issue here.

1 option I see is to compile the scss to css myself, and include that file in my project.
But not sure if that is the best approach when using Astro...

@bluwy
Copy link
Member

bluwy commented Sep 22, 2023

I'm seeing the Vite injected styles and Astro injected styles are strangely conflicting too. Would be great if you can open another issue for this 🙏

@marceloverdijk
Copy link
Author

Thx for following up @bluwy ; I have created #8637

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) feat: styling Related to styles (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants