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

:global having only 1 selector is a BREAKING change #6477

Closed
snird opened this issue Jun 29, 2021 · 10 comments · Fixed by #6508
Closed

:global having only 1 selector is a BREAKING change #6477

snird opened this issue Jun 29, 2021 · 10 comments · Fixed by #6508
Labels
awaiting submitter needs a reproduction, or clarification bug compiler Changes relating to the compiler

Comments

@snird
Copy link

snird commented Jun 29, 2021

Describe the bug

this: 8c3fb92

this breaks usable and known behavior.

Reproduction

every :global(.class1 .class2) or similar is now broken.

Logs

No response

System Info

npx: installed 1 in 0.816s

  System:
    OS: Linux 5.4 Ubuntu 20.04 LTS (Focal Fossa)
    CPU: (16) x64 AMD Ryzen 7 5800X 8-Core Processor
    Memory: 8.41 GB / 15.61 GB
    Container: Yes
    Shell: 5.0.16 - /bin/bash
  Binaries:
    Node: 14.17.0 - ~/.nvm/versions/node/v14.17.0/bin/node
    npm: 6.14.13 - ~/.nvm/versions/node/v14.17.0/bin/npm
  npmPackages:
    svelte: ^3.38.3 => 3.38.3

Severity

blocking all usage of svelte

@snird
Copy link
Author

snird commented Jun 29, 2021

@ignatiusmb letting you know

@pngwn pngwn added bug compiler Changes relating to the compiler awaiting submitter needs a reproduction, or clarification labels Jun 29, 2021
@Conduitry
Copy link
Member

For background: This was originally disallowed because #5907 was opened asking for different behavior in this situation that we didn't want to allow, and so we decided to make it a compiler error rather than have confusing behavior in that situation.

@ignatiusmb
Copy link
Member

Thanks for letting me know. That commit is still unreleased, and your example :global(.class1 .class2) would still work, it will only throw a compile error on grouping selectors, which is only on selector list. Any other selector combinations should not be affected.

I see you're using the latest version, if what you meant is v3.38.3, then #6428 is the corresponding PR which is opened to close the issue linked above. I don't think it is necessarily "blocking all usage of svelte", and I think it's a good decision to disallow it as it doesn't really match what we expect as described in the issue. Although, I do agree that it might better released as v3.39.0 as it breaks most v3.38.x apps.

@jacobmischka
Copy link
Contributor

In true semantic versioning it should have been a v4 change, but yes at least certainly a minor version if the major version is tied to marketing.

@snird
Copy link
Author

snird commented Jun 30, 2021

It broke my usage of it, which isn't multiple selectors but refining a selector.
e.g .content .title.

I'm not even sure this PR intended to error on this kind of usage

@dummdidumm
Copy link
Member

Please provide a reproducible code snippet with a complete CSS selector, not just parts of it. Both your given snippets don't give an error for me, so there must be more to it.

@jacobmischka
Copy link
Contributor

I can confirm child selectors like above still work for me on the latest version as well.

@infuzz
Copy link
Contributor

infuzz commented Jul 2, 2021

I confirm that this change throw an error when using MULTIPLE selectors in a list comma separated like this:
:global( div, p)
https://svelte.dev/repl/2eacdbae6cdc4a9f906516a9dbe7936d?version=3.38.3

Until 3.38.2 (https://svelte.dev/repl/2eacdbae6cdc4a9f906516a9dbe7936d?version=3.38.2) it was working well. This is breaking change for me since I use this in many components )-; and the doc does not specify this limitation.

@dummdidumm
Copy link
Member

Your given code snippet is indeed something that would have worked correctly before and gave correct output. So I think we should - for semver reasons - revert the change for this specific case where the css selectors consists of a single :global only. @tanhauhau @Conduitry thoughts?

@Conduitry
Copy link
Member

The specific case where :global() is the only thing in its selector is again allowed to contain multiple selectors in 3.39.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting submitter needs a reproduction, or clarification bug compiler Changes relating to the compiler
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants