dperini / nwsapi

Fast CSS Selectors API Engine
MIT License
103 stars 35 forks source link

Upgrade from nwsapi 2.2.2 to 2.2.3 creates infinite loop #80

Closed cmdcolin closed 1 year ago

cmdcolin commented 1 year ago

Hello there, I found that upgrading nwsapi from 2.2.2 to 2.2.3 created an infinite loop in a repo that runs jest/jsdom/testing-library on a page that uses @mui/x-data-grid. I isolated this infinite loop to the nwsapi version bump.

Here is a screenshot of the code that gets infinite looped

image

code from screenshot pasted below here

(function anonymous(s) {
    "use strict";
    return function Resolver(c, f, x, r) {
        var e, n, o;
        e = c;
        n = s.doc.activeElement;
        while (e) { // <<<----- stuck in this loop infinitely
            if (e === n || e.parentNode === n)
                break;
        }
        if ((e === n && s.doc.hasFocus() && (e.type || e.href || typeof e.tabIndex == "number"))) {
            if ((/(^|\s)MuiDataGrid-columnHeader(\s|$)/.test(e.getAttribute("class")))) {
                var N2 = e;
                while (e && (e = e.parentElement)) {
                    if ((/(^|\s)css-1e2bxag-MuiDataGrid-root(\s|$)/.test(e.getAttribute("class")))) {
                        r = true;
                    }
                }
                e = N2;
            }
        }
        return r;
    }
}
)

I am not sure exactly how it generates this code, but it seems to me that it would be easy for this loop to never end (and indeed, it does not end in my code) as e never changes in this loop?

I made a 'minimal' reproduction here https://github.com/cmdcolin/mui_datagrid_jest_infinite_loop/, it is a create-react-app instance

steps to repro

git clone https://github.com/cmdcolin/mui_datagrid_jest_infinite_loop/
cd mui_datagrid_jest_infinite_loop
git checkout works
yarn
yarn test # passes
git checkout fails
yarn
yarn test # infinite loop

the only difference between these branches is the nwsapi version bump

If you think this is something to report to a different repository (e.g. jsdom or testing-library or something), let me know

dperini commented 1 year ago

@cmdcolin thank you for the prompt detection of this bug (:focus-within related). I am going to push a tentative fix for this on the main github repository. Can you apply that in your testing environmen and check if this change resolves ?

cmdcolin commented 1 year ago

Appears fixed after linking the latest master branch with my test environment!

cmdcolin commented 1 year ago

can probably close now

cmdcolin commented 1 year ago

note that 2.2.4 might not be on npm yet, though it appears tagged on master branch

janpe commented 1 year ago

@cmdcolin thank you for the prompt detection of this bug (:focus-within related). I am going to push a tentative fix for this on the main github repository. Can you apply that in your testing environmen and check if this change resolves ?

@cmdcolin do you have a plan on when you're going to release this fix? Also, should 2.2.3 be removed from the npm registry since it's broken and already has over one million downloads.

cmdcolin commented 1 year ago

just to be clear, I am not in charge of the release process :) I believe that would be @dperini

jahumes commented 1 year ago

As another data point, this bug also affected our code when using window.getComputedStyles inside of jest/jsdom. It returned an infinite loop in our tests which took several hours to track down. @dperini Please remove 2.2.3 from NPM since it has over 1 million downloads in the last week. It took a lot of work to track down since it is a dependency of a dependency.

If this had been released promptly, it would have saved us a lot of headaches 😄

dperini commented 1 year ago

A fixed 2.2.4 is on its way to npm in a few moments, the fix was not trivial it is a new attempt to solve different problems parsing edge cases related with the :not pseudo-class, I hope the wait was worth it.

dperini commented 1 year ago

@cmdcolin I will appreciate one more test from you confirming the problems detected is now resolved in release 2.2.4. Thank you.

cmdcolin commented 1 year ago

@dperini yes, the now published v2.2.4 fixes it for me! thanks much :)

jahumes commented 1 year ago

@dperini Thanks for responding so promptly! v2.2.4 has fixed the issue for me as well.

sagar1111212121 commented 1 year ago

Not similar, but after upgrading to latest 2.2.6, JEST + RTL test cases started failing which were working just fine before sometime. Anyone else facing this issue?