Prevent invalid-regular expression syntax error in Safari < 16.4

[tf]

PR #177 added regular expressions to parse CSSNumericValue objects using lookbehind. Safari only supports lookbehind starting from version 16.4 [1]. In earlier versions loading the polyfill causes an error of the form:

SyntaxError: Invalid regular expression: invalid group specifier name

Compile RegExp on demand to ensure the polyfill can be loaded without errors. Since the relevant functions are only called initially when parsing options, the performance overhead appears limited.

[1] https://caniuse.com/js-regexp-lookbehind

[tf]

Happy to consider caching the regexps if I'm missing use cases where the performance overhead of recompiling each time might become relevant. Shortly considered alternative ways to formulate the expressions without lookbehind, but couldn't think of anything that wouldn't make the code much more complex.

[bramus]

Shortly considered alternative ways to formulate the expressions without lookbehind, but couldn't think of anything that wouldn't make the code much more complex.

I think this would be better to do, as it actually solves the underlying problem.

[tf]

I agree that this would be the nicer solution, but the absence of unit tests makes the parsing logic rather scary to change. I played a bit with the current implementation and am still unsure which cases it is supposed to handle and if it does so correctly.

[johannesodland]

I'm sorry for adding lookbehind without properly checking support.

I needed an implementation of CSSNumericValue.parse() that could parse simple math functions such as calc((1px + 2px) * 3), and some of the scroll-timelines subtests require CSSNumericValue.parse() to run. The lookbehind regex is used to split expressions such as (1px + 2px) * 3 into 1px + 2px and 3.

I agree that it would be better to add a proper implementation of CSSNumericValue.parse(). There are web platform tests, such as parse.tentative.html that can be used to test the method.

(Note that these tests fail if the the build is run with terser so it's necessary to build with the --no-compress flag, see https://github.com/flackr/scroll-timeline/pull/200)

Until we have a proper implementation, we could detect lookbehind-support and throw an error if a math-function is provided in parseCSSNumericValue: https://github.com/flackr/scroll-timeline/blob/147d187620e49ce5fb3abc9e1c3e1018755d8f02/src/proxy-cssom.js#L177-L184

[tf]

we could detect lookbehind-support

The problem is that using lookbehind in a literal is a syntax error that cannot be caught in JS. This is why I proposed the workaround to use dynamically compiled expressions.

[johannesodland]

I implemented tokenization and parsing for CSSNumericValue.parse() in https://github.com/flackr/scroll-timeline/pull/206 if we want to go that route. Looking forward to having parse() in all browsers so we don't have to reimplement it when polyfilling 😅.

This will still leave one regexp in parseInset() where we need to consume a list of component values.

[johannesodland]

I added a PR for a utility function splitIntoComponentValues() in https://github.com/flackr/scroll-timeline/pull/209 that would remove the need for the final lookahead regex in parseInset().

[johannesodland]

All lookahead regexes have been replaced by proper parsing now. Is it ok to close this?

[bramus]

When it comes to CSS, doing actual parsing indeed beats regexes. I think it’s fine to close this one indeed :)