nwsapi icon indicating copy to clipboard operation
nwsapi copied to clipboard

h1 + h2 appears to break

Open Fil opened this issue 1 year ago • 4 comments

This selector errors after c104f8cdf49d38699a9570d327446689251efe11; it's working with the parent 34c2154acbaf0e2b21accecbb1bf9c71fc5ee229

Dom.select("h1 + h2")

Fil avatar May 13 '24 12:05 Fil

@Fil ... yes the last commit brings a regression within the changes made to improve and further isolate HTML / XML / XHTML languages inside "nwsapi" so next cut release will not contain this last namespaces related commit. Hope no more regression will surface. I understand developers need a "new stable nwsapi" release library and I will now focus on making that happen. Thank you for the help.

dperini avatar May 14 '24 20:05 dperini

Just in case it helps, here's a more intricate case. It's a different error, but I was getting 'not valid selector' before, so I guess it's related.

selector: ':is(.a, .b:not(.c:is(.d), .e), .f), .g'

stacktrace:

SyntaxError: unknown pseudo-class selector ':not(.c:is(.d'
    at emit (\node_modules\.pnpm\[email protected]\node_modules\nwsapi\src\nwsapi.js:576:17)
    at compileSelector (\node_modules\.pnpm\[email protected]\node_modules\nwsapi\src\nwsapi.js:1300:17)
    at compile (\node_modules\.pnpm\[email protected]\node_modules\nwsapi\src\nwsapi.js:766:16)
    at match_collect (\node_modules\.pnpm\[email protected]\node_modules\nwsapi\src\nwsapi.js:1371:16)
    at Object._matches [as match] (\node_modules\.pnpm\[email protected]\node_modules\nwsapi\src\nwsapi.js:1426:35)
    at Array.Resolver (eval at compile (\node_modules\.pnpm\[email protected]\node_modules\nwsapi\src\nwsapi.js:781:17), <anonymous>:3:104)
    at collect (\node_modules\.pnpm\[email protected]\node_modules\nwsapi\src\nwsapi.js:1578:21)
    at Object._querySelectorAll [as select] (\node_modules\.pnpm\[email protected]\node_modules\nwsapi\src\nwsapi.js:1533:36)

miguel-leon avatar May 25 '24 01:05 miguel-leon

@miguel-leon yes it is related ... I almost fixed the spill. A mix up of wrong RegExps changes brought me here. The ability of parsing nested selectors compounds was not the objective of "nwsapi", not now. It will be available soon I hope, but not yet, not for a bug free release bug free at least, but it has become the time to close this circle and push a working release.

The problem is known ... in this moment compiler generated code is not always up to the complexity of the task in term of nesting, levels deep. It has worked in "nwmatcher" why should it fail in "nwsapi" ? Well ... first because I missed up a particular parameter where the errors should be also parsed/evaluated by looping through alle the sub-nested exceptions.

I am working to push a commit for this failure only and see how this affects remaining problems.

dperini avatar May 25 '24 11:05 dperini

Oh. If the problem is RegExps, it's annoying because they lack recursion. So also, in case it helps, you could decide and document that nesting is supported only up to a certain depth, and then repeat the expression inside of itself a couple of times to circumvent the recursion problem without using another mechanism outside the realm of regexps to loop through sub-nested cases.

For example for parenthesis:

  • Depth 1
\((?:[^)])*\)
  • Depth 2
\((?:\((?:[^)])*\)|[^)])*\)
  • Depth 4
\((?:\((?:\((?:\((?:[^)])*\)|[^)])*\)|[^)])*\)|[^)])*\)
  • Depth 8
\((?:\((?:\((?:\((?:\((?:\((?:\((?:\((?:[^)])*\)|[^)])*\)|[^)])*\)|[^)])*\)|[^)])*\)|[^)])*\)|[^)])*\)|[^)])*\)

✌️

miguel-leon avatar May 25 '24 19:05 miguel-leon

Wrong Regular expression caused this. Closing as completed. Reopen if necessary.

dperini avatar Mar 18 '25 18:03 dperini

Hi, I didn't understand your last message. Do you mean the problem was due to bad regular expressions and now is fixed? Or do you mean that you think that there's nothing to fix? Thanks

miguel-leon avatar Mar 18 '25 18:03 miguel-leon

@miguel-leon
regarding the resolver issue h1 + h2 appears to break it was fixed. It had to do with wrong REs and other overlapping problems, not performing in the intended way due to a problem related to how I wrote them.

More generally at that time I didn't have enough time to investigate further on the bespoken RE recursion. So I went for the standard way I used previously, thus handling the recursion with Javascript through sub-nesting and matching the strings at each level with REs, a method you also describe in your message above.

Now that those problems have been fixed and I find myself in a less stressing position I will have the opportunity and the needed time to investigate this problem in the right way and adopt an alternate strategy for these resolvers.

Previously, in your message, you wrote:

If the problem is RegExps, it's annoying because they lack recursion.

It turns out that PCRE 4.0 already implements recursion natively and that these capabilities are exactly what we need.

So I am now working on code to feature detect availability of recursion in current Regular Expression implementations inside browsers and it looks like the great majority of today browsers natively support PCRE 4.0 and recursion.

dperini avatar Mar 19 '25 00:03 dperini