fix: parser error when using semicolon inside quotes in style #10221
Conversation
🦋 Changeset detectedLatest commit: a7226db The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
n0n3br
changed the title
| @@ -9,7 +9,7 @@ const REGEX_PERCENTAGE = /^\d+(\.\d+)?%/; | |||
| const REGEX_NTH_OF = | |||
| /^(even|odd|\+?(\d+|\d*n(\s*[+-]\s*\d+)?)|-\d*n(\s*\+\s*\d+))((?=\s*[,)])|\s+of\s+)/; | |||
| const REGEX_WHITESPACE_OR_COLON = /[\s:]/; | |||
| const REGEX_BRACE_OR_SEMICOLON = /[{;]/; | |||
| const REGEX_BRACE_OR_SEMICOLON = /[{;](?=(?:[^']*'[^']*')*[^']*$)/; | |||
There was a problem hiding this comment.
| const REGEX_BRACE_OR_SEMICOLON = /[{;](?=(?:[^']*'[^']*')*[^']*$)/; | |
| const REGEX_BRACE_OR_SEMICOLON = /[{;](?=[^"']*$)/; |
I think the extra complexity here isn't needed — just checking that it doesn't end with a quote would be sufficient. Also don't forget the " character.
There was a problem hiding this comment.
Sorry about testing, I'm starting in this world of contributing to open source now and even don't know how I would write a test for that.
As for the changeset, is there a style guide or other PR that I could use as example ?
There was a problem hiding this comment.
No worries! The test case for this should be added in parser-modern. You can read more about contributing to Svelte here.
Unfortunately, I can't help you add a test case as I don't have write access to this PR. You can look at similar tests inside parser-modern though to understand how it works. In short, it is a snapshot test, meaning it takes an input and expects a certain output, if the generated output is different than the expected it fails.
There was a problem hiding this comment.
@navorite added a changeset and commited your sugestion. as for tests I'm still stucked
Co-authored-by: navorite <navorite@gmail.com>
playgrounds/demo/demo.css
Outdated
There was a problem hiding this comment.
Revert this change, it's not relevant to the PR at all. Wait until hopefully a maintainer with write access can add a test to your PR.
There was a problem hiding this comment.
I added a test in the folder you told me. Hope I did it right. Thanks for your help
There was a problem hiding this comment.
Sorry about the unecessary file change. Reverted it. Thanks for your help again.
There was a problem hiding this comment.
@navorite just fixed the support-font-face test not passing. Had to revert the regex to the first one I've used. Your suggestion breaks the test. Already commited. 100% of tests are passing now.
Co-authored-by: navorite <navorite@gmail.com>
|
Thanks! This wasn't quite the right fix — it would basically match any semi/brace that didn't have quote marks later in the string, which meant incorrect results like this: 
We just need to parse it properly, one character at a time. Luckily we already have the code to do that, it just needed a very modest tweak |
* fix error when using semicolon inside quotes in style
* refactor to include { like in the original code
* simplified version of regex
Co-authored-by: navorite <navorite@gmail.com>
* add changeset
* add changeset
* add test
* Update .changeset/seven-hornets-smile.md
Co-authored-by: navorite <navorite@gmail.com>
* undo demo.css change
* fix support-font-face test not passing
* add double quotes
* beef up test
* robustify parsing
* Update .changeset/seven-hornets-smile.md
---------
Co-authored-by: navorite <navorite@gmail.com>
Co-authored-by: Rogerio Luiz Aques de Amorim <Rogerio Amorim>
Co-authored-by: Rich Harris <rich.harris@vercel.com>
Co-authored-by: Rich Harris <hello@rich-harris.dev>
fix #9637
This PR fixes the parsing error when we use semicolon inside quotes in style tag, like described in the issue.