svelte icon indicating copy to clipboard operation
svelte copied to clipboard

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

Open dummdidumm opened this issue 1 year ago • 8 comments

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

dummdidumm avatar Oct 21 '24 21:10 dummdidumm

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

paoloricciuti avatar Oct 21 '24 21:10 paoloricciuti

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

dummdidumm avatar Oct 22 '24 07:10 dummdidumm

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)

paoloricciuti avatar Oct 22 '24 07:10 paoloricciuti

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.

Conduitry avatar Oct 22 '24 12:10 Conduitry

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.

dominikg avatar Oct 22 '24 12:10 dominikg

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

dummdidumm avatar Oct 22 '24 19:10 dummdidumm

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.

Conduitry avatar Oct 22 '24 19:10 Conduitry

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

dummdidumm avatar Oct 22 '24 19:10 dummdidumm