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

fix(core): fix for svelte extractor #1830

Merged
merged 8 commits into from Nov 2, 2022

Conversation

chu121su12
Copy link
Collaborator

@chu121su12 chu121su12 commented Nov 2, 2022

Should close #1828

Caveat: this would make svelte extractor not supporting data-x and aria-x variants directly.

@netlify
Copy link

netlify bot commented Nov 2, 2022

Deploy Preview for unocss ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit ebf36ec
🔍 Latest deploy log https://app.netlify.com/sites/unocss/deploys/63629cfd22bf080009873fee
😎 Deploy Preview https://deploy-preview-1830--unocss.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@chu121su12 chu121su12 marked this pull request as ready for review November 2, 2022 00:37
@antfu
Copy link
Member

antfu commented Nov 2, 2022

Does that mean in Svelte you can't use rules with =? Would that be a bit confusing and inconsistent?

@chu121su12
Copy link
Collaborator Author

The two variants if not using theme mostly depends on the = in attribute selector (ex.: data-[a~=b]:rule). And since the update brings back splitting on =, rules with = may never get matched directly; except when manually generated via uno.generate().

Although if the syntax is per the issue class:text-orange-400={foo}, I think we can do left + right trim, of ex. /class:(.*)\s*=\s*/, as another workaround; due to the other split will happen at the character {. The extractor already split and do the left trim:

return r.startsWith('class:') ? r.slice(6) : r

@chu121su12
Copy link
Collaborator Author

Added the right trim as of a36f54b. @pheuter If you don't mind, please check the syntax usage in the test.

@pheuter
Copy link

pheuter commented Nov 2, 2022

Looks about right to me 👀

The particular example in my issue is prob the most common, but not exhaustive of the possible syntax that can be parsed, like this one that currently works too for example:

<div class:text-[32px]={true} />

@chu121su12
Copy link
Collaborator Author

I'm sorry I don't know svelte syntax much. If the syntax is abc={def without any spaces then the latest update should work the same. If there's more combination other than the ={, it may warrant another workaround.

@pheuter
Copy link

pheuter commented Nov 2, 2022

The only other separator that comes to mind is ="{}" in addition to ={}

@chu121su12
Copy link
Collaborator Author

="{}" should be processed the same way as ={}

@antfu antfu merged commit 52a71f4 into unocss:main Nov 2, 2022
@chu121su12 chu121su12 deleted the fix/svelte-regression branch January 9, 2023 05:21
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 this pull request may close these issues.

Svelte class: regression in 0.46.2
3 participants