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

Migration: Wrap :has/not/where/is(...) selectors in :global(...) if necessary #13765

Closed
dummdidumm opened this issue Oct 21, 2024 · 8 comments · Fixed by #13850
Closed

Migration: Wrap :has/not/where/is(...) selectors in :global(...) if necessary #13765

dummdidumm opened this issue Oct 21, 2024 · 8 comments · Fixed by #13850

Comments

@dummdidumm
Copy link
Member

Describe the problem

These are scoped in Svelte 5, but weren't in Svelte 4

Describe the proposed solution

Adjust migration script to add global if necessary

Importance

nice to have

@paoloricciuti
Copy link
Member

What do we do if we have this AND postcss? Bail out and add a migration task?

@dummdidumm
Copy link
Member Author

What does postcss change about this? Can you give an example?

@paoloricciuti
Copy link
Member

What does postcss change about this? Can you give an example?

That we are blanking out css in the migration script because it was erroring out (i'm assuming because svelte is not able to parse it without a preprocessor)

@Conduitry
Copy link
Member

This is something that people need to change even in non-runes mode, right? Should this be part of a separate class of (simpler) migrations that people need to perform upon upgrading to v5, rather than the more complex and fiddly and error-prone ones involving switching to runes?

See also #11313, which feels like something that a migration would be nice for, but which should happen along with this :has() etc. change, and not along with switching to runes.

@dominikg
Copy link
Member

What does postcss change about this? Can you give an example?

i'm assuming because svelte is not able to parse it without a preprocessor

uhh we do have the preprocessors of the project available and can use them, right? (just import svelte.config.js and use preprocess). that might not give us the input css but the output css should be parsable by svelte and then we can inspect it to learn if it uses a feature that needs user attention and add a migration marker comment to that style block without nuking its source.

@dummdidumm
Copy link
Member Author

I'm confident a conservative regex will get us far enough

@Conduitry
Copy link
Member

Does anyone have any thoughts about breaking up migrations into two phases: 'needs to be done in non-runes mode due to breaking changes' and 'can be done later as you gradually switch to runes, and more likely to need manual attention'? It feels weird, but I don't have another great solution here. Combining obligatory non-runes changes together with optional runes rewrites into a single migration script rubs me the wrong way.

@dummdidumm
Copy link
Member Author

I think it's a good point, and my proposal is to make this an option in our migration function, and we can then prompt in the migration script "convert components to new syntax?" with the default being Yes

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 a pull request may close this issue.

4 participants