Regression: Svelte `:global()` parsing doesn't play nicely with SASS parent selectors: &[attr='value']

TylerRick commented 2 years ago

Describe the bug

Background

When using svelte with svelte-preprocess and sass, it should be possible to use SASS parent selector & in the way we're used to, with the same semantics of being able to "take the parent selector and add something more specific to it" (my paraphrase).

Extending the semantics of SASS & to the Svelte world of :global()ness... It would seem to me that if a & occurs within a block that is already :global(), that the globalness of the resulting CSS selector should be preserved. Shouldn't it?

(Which has me wondering if this should actually be the responsibility of svelte-preprocess, which is supposed to be what takes SCSS input, and while preserving its meaning/intent, translates it to "bare Svelte" syntax that Svelte can understand.)

For example, let's say we have a list, where each element has a unique class or attribute...

<ul>
  <li data-i="0">...</li>
  <li data-i="1">...</li>
</li>

We may want to use some SCSS like this to group them together and style each of those unique elements...

li {
  /* Styles common to all li elements. And then styles for unique elements... */
  &[data-i='0'] {
    background-color: red;
  }
  &[data-i='1'] {
    background-color: blue;
  }
}

This produces the following CSS:

li {
  /* Styles common to all li elements. */
}
li[data-i="0"] {
  background-color: red;
}
li[data-i="1"] {
  background-color: blue;
}

The Problem

Until I upgraded svelte recently, this was working as expected in my Svelte components, too:

<ul>
  <Item data-i="0">...</Item>
  <Item data-i="1">...</Item>
</li>

<style lang="scss">
:global(li) {
  &[data-i='1'] {
    background-color: blue;
  }
}
</style>

In Svelte 3.37.0, this produced a <style> tag with this CSS:

li[data-i="1"]{background-color:blue}

It wasn't necessary to wrap the parent selector (&[data-i='1']) with :global(). In fact, doing so actually broke things because it caused the & to come through as-is, which is invalid CSS:

li &[data-i="1"]{background-color:blue}

Fast-forward to Svelte 3.38.0 or later, and we get somewhat opposite behavior and a breaking change... Try to keep doing this:

:global(li) {
  &[data-i='1'] {
    background-color: blue;
  }
}

and you get scolded:

Unused CSS selector ":global(li)[data-i="1"]"

and the selector removed.

Try to change it to this:

:global(li) {
  :global(&[data-i='1']) {
    background-color: blue;
  }
}

and you get the same invalid CSS as previous versions:

li &[data-i="1"]{background-color:blue}

Either way, it doesn't work! (Meaning, it broke my component when I upgraded.)

Workarounds

Of course I searched for a workaround...

Workaround 1: Don't use SASS — or at least not & — when dealing with a :global() style.

Obviously this works...

:global(li[data-i='1']) {
    background-color: blue;
  }
}

but I'd prefer to use &.

Workaround 2: Use :global {} feature from svelte-preprocess

... which lets you mark entire blocks as global (which is pretty useful, and preferable to workaround 1):

:global {
  li {
    &[data-i='1'] {
      background-color: blue;
    }
  }
}

Root cause

The only documented change to :global() behavior between 3.37.0 and 3.38.0 was #6222, so of course I assume that changeset (#6223) is what inadvertently broke this for SASS users.

What I would like to do is p:global(.xxx), while still scoped to p within the component, it will treat .xxx as global (ie, do not go and match any selector written within :global())

I don't quite understand #6222 enough to say why it should have broken it for this use case, but I would guess that in order to make it still add a .svelte-xyz scoping class for cases like this:

    div:global(.bar) {
        color: red;
    }

and result in this (from this diff):

div.svelte-xyz.svelte-xyz.bar{color:red}

, it was necessary to change the logic to add the .svelte-xyz part if any part of a selector did not have a :global(). And in my examples, the [attr] that svelte-preprocess currently gives us is outside of a :global().

Indeed, it looks like the root cause of this breaking change might go back as far as 3.18.1 and https://github.com/sveltejs/svelte/issues/4314#issuecomment-579011806:

As mentioned above, you need to use :global() around any portion of the selector that doesn't refer to this component, whether it's above us or below us in the tree.

Proposed solutions

I'm not entirely sure yet whether a solution for this would belong in...

svelte-preprocess (in order to preserve the intended globalness of the original SASS input when converting to CSS output that Svelte will understand)
or here in svelte (to be more tolerant of certain CSS input)

I noticed that among the use cases that #6223 was adding support for:

    div:global(.bar) {
        color: red;
    }

    .foo:global(.bar) {
        color: red;
    }

, none of them involve a [attr] selector.

Is it possible that changing it so that it doesn't add the .svelte-xyz to selectors that contain only :global(...)[attr] would be enough to solve thing both for the #6223 use cases and the SASS use case presented here?

Then again, I might argue that even if you add a class to a &, that the intended/desired outcome from a SASS user would be that anything within a :global(anything) { block stays global:

:global(li) {
  &.item1 {
    background-color: red;
  }
  &.item2 {
    background-color: blue;
  }
}

It's open to interpretation. But if we interpret the & as saying "exactly the same as the parent selector, but with...", then it should preserve the globalness and give us :global(li.item1).

Does that sound right?

And is it even possible to support that while also supporting the intent of #6222?

(I see a related report from a SASS user here about a regression from 3.37.0 that also involves an [attr] selector, :global(div[row=one], but that's not quite the same problem since they actually get an error in that case:

global(...) can be at the start or end of a selector sequence, but not in the middle

)

Reproduction

https://github.com/TylerRick/svelte-sass_global_attr_regression

Logs

Unused CSS selector ":global(li)[data-i="1"]"

Severity

blocking an upgrade

elron commented 1 year ago

This problem still exists

thnee commented 11 months ago

Use case: Have element with class column defined in svelte template. Add class moving dynamically to it from a JS event. Add style to the element that has both classes.

Not sure what is supposed to work or not, so I tested the following:

This works.

:global(.column.moving) {
    border: 1px solid green;
}

This works.

.column {
    &:global(.moving) {
        border: 1px solid green;
    }
}

This does not work.

.column {
    :global(&.moving) {
        border: 1px solid green;
    }
}

This does not work.

:global(.column) {
    &:global(.moving) {
        border: 1px solid green;
    }
}

This does not work.

:global(.column) {
    :global(&.moving) {
        border: 1px solid green;
    }
}

This does not work.

:global(.column) {
    &.moving {
        border: 1px solid green;
    }
}

This does not work.

:global() {
    .column {
        &.moving {
            border: 1px solid green;
        }
    }
}

Using svelte 4.2.7 and SvelteKit 1.27.4.

matiboux commented 6 months ago

Still not working in Svelte 4.2.15 and SASS 1.76.0.
Tested in Astro 4.7.1 with @astrojs/svelte 5.4.0.

Examples that does not work:

⚠️ This generates CSS .column:global(.moving){border: 1px solid green}:
```
:global(.column) {
&:global(.moving) {
    border: 1px solid green;
}
}
```

⚠️ This generates CSS .column &.moving{border: 1px solid green}:

:global(.column) {
:global(&.moving) {
    border: 1px solid green;
}
}

❌ This triggers the error :global(...) not at the start of a selector sequence should not contain type or universal selectors:
```
:global(.column) {
&:global(&.moving) {
    border: 1px solid green;
}
}
```

⚠️ This generates nothing:

:global(.column) {
&.moving {
    border: 1px solid green;
}
}

dummdidumm commented 1 month ago

Would appriciate if someone could check if this is still happening with Svelte 5, or if it's fixed there.

sveltejs / svelte