fix: do not consume number token if expression ends with '.' #164
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`.
| @@ -44,7 +44,7 @@ | |||
| (([0-9]+("."[0-9]+)?|"."[0-9]+)(e(\+|-)[0-9]+)?)dpcm\b return 'RES'; | |||
| (([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
| @@ -44,7 +44,7 @@ | |||
| (([0-9]+("."[0-9]+)?|"."[0-9]+)(e(\+|-)[0-9]+)?)dpcm\b return 'RES'; | |||
| (([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'; | |||
|
|
|||
| (([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
Fix #132
\balso matches '.', so the regex for the number expression would match41in41.7rpx. ThenUNKNOWN_DIMENSIONwould match.7rpxand create the invalid sequenceNumberExpression,UnknownDimension.