Skip to content

fix: do not wrongly suggest that a lang attribute is missing when finding comments #1166

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

Merged
merged 1 commit into from
Jul 10, 2025

Conversation

maxlath
Copy link
Contributor

@maxlath maxlath commented Jul 9, 2025

Additionally to the issue reported in #1165, enhanceCompileError was wrongly reporting that some of my <style> blocks were missing a lang attribute, due to the styleRe also matching comment block. In This PR, I thus suggest to not match comment blocks for that purpose.

@dominikg
Copy link
Member

dominikg commented Jul 9, 2025

as far as i remember this regex is coming from https://github.com/sveltejs/svelte/blob/9926347ad9dbdd0f3324d5538e25dcb7f5e442f8/packages/svelte/src/compiler/preprocess/index.js#L257 and the part you removed here is used to consume comments to avoid processing style blocks inside comments.

do you have a reproduction where a commented style is matched and processed by this? kind of reluctant to diverge from sveltes regex here (ultimately i'd love to get rid of it entirely, it is much too complex)

@maxlath
Copy link
Contributor Author

maxlath commented Jul 9, 2025

I pushed a fixup commit 69a2e0b that would preserver the regex, and here is a demo of the issue https://github.com/maxlath/vite-plugin-svelte-demo/

@dominikg
Copy link
Member

dominikg commented Jul 9, 2025

Thank you for working on this! to be able to release it we have to add a changeset. you can do that by running pnpm changeset and following the prompts. (select vite-plugin-svelte, patch and add a short description)

@maxlath maxlath force-pushed the fix-enhanceCompileError branch from 743563c to 0e1a460 Compare July 9, 2025 14:46
@maxlath
Copy link
Contributor Author

maxlath commented Jul 9, 2025

I ran pnpm changeset, squashed the fixup commits, and force pushed

@dominikg
Copy link
Member

dominikg commented Jul 9, 2025

Thanks, i'm in the middle of preparing 6.0.0, so this might go out after as 6.0.1 to not derail the already generated changelogs etc.

@dominikg dominikg merged commit 666dc61 into sveltejs:main Jul 10, 2025
8 checks passed
@github-actions github-actions bot mentioned this pull request Jul 10, 2025
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.

2 participants