fix: do not consume number token if expression ends with '.'#164
fix: do not consume number token if expression ends with '.'#164ludofischer wants to merge 1 commit intomasterfrom
Conversation
Fix #132 `\b` also matches '.', so the regex for the number expression would match `41` in `41.7rpx`. Then `UNKNOWN_DIMENSION` would match `.7rpx` and create the invalid sequence `NumberExpression`, `UnknownDimension`.
ludofischer
left a comment
ludofischer
left a comment
There was a problem hiding this comment.
This solves the issue but maybe it's better to refactor so that numbers and units are parsed as separate tokens, which is closer to what the CSS spec says.
| (([0-9]+("."[0-9]+)?|"."[0-9]+)(e(\+|-)[0-9]+)?)dppx\b return 'RES'; | ||
| (([0-9]+("."[0-9]+)?|"."[0-9]+)(e(\+|-)[0-9]+)?)\% return 'PERCENTAGE'; | ||
| (([0-9]+("."[0-9]+)?|"."[0-9]+)(e(\+|-)[0-9]+)?)\b return 'NUMBER'; | ||
| (([0-9]+("."[0-9]+)?|"."[0-9]+)(e(\+|-)[0-9]+)?)(?!".")\b return 'NUMBER'; |
There was a problem hiding this comment.
Using a lookahead assertion to check there is no dot before the word boundary. That's illegal CSS anyway https://drafts.csswg.org/css-syntax-3/#typedef-number-token
| (([0-9]+("."[0-9]+)?|"."[0-9]+)(e(\+|-)[0-9]+)?)\b return 'NUMBER'; | ||
| (([0-9]+("."[0-9]+)?|"."[0-9]+)(e(\+|-)[0-9]+)?)(?!".")\b return 'NUMBER'; | ||
|
|
||
| (([0-9]+("."[0-9]+)?|"."[0-9]+)(e(\+|-)[0-9]+)?)-?([a-zA-Z_]|[\240-\377]|(\\[0-9a-fA-F]{1,6}(\r\n|[ \t\r\n\f])?|\\[^\r\n\f0-9a-fA-F]))([a-zA-Z0-9_-]|[\240-\377]|(\\[0-9a-fA-F]{1,6}(\r\n|[ \t\r\n\f])?|\\[^\r\n\f0-9a-fA-F]))*\b return 'UNKNOWN_DIMENSION'; |
There was a problem hiding this comment.
Alternatively we could try simplifying UNKNOWN_DIMENSION. Why does it need to match so many types of characters, even \r, \t, etc? According to the spec, the unit can only be a CSS identifier https://www.w3.org/TR/css-values-3/#css-identifier
There was a problem hiding this comment.
I think it is right direction, yes, I think it is wrong
There was a problem hiding this comment.
I realized that this parsing duplicates a lot of work that's already done in postcss-value-parser. It calls postcss-value-parser, stringifies the result, then parses again. Looks like the reason is that this package also wants to parse calc() in selectors, which is not in the spec. But it feels a bit like a waste to do this work all over again. There's already 3 separate CSS tokenizers in postcss, postcss-value-parser, and postcss-selector-parser. So together with postcss-calc, the tokenizer has been reimplemented 4 times in PostCSS projects.
There was a problem hiding this comment.
Yes, It was here before I was invited and fix bugs, will be great to rewrite it on one parser
2 participants
Fix #132
\balso matches '.', so the regex for the number expression would match41in41.7rpx. ThenUNKNOWN_DIMENSIONwould match.7rpxand create the invalid sequenceNumberExpression,UnknownDimension.