Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade from nwsapi 2.2.2 to 2.2.3 creates infinite loop #80

Closed
cmdcolin opened this issue Apr 8, 2023 · 12 comments
Closed

Upgrade from nwsapi 2.2.2 to 2.2.3 creates infinite loop #80

cmdcolin opened this issue Apr 8, 2023 · 12 comments

Comments

@cmdcolin
Copy link

cmdcolin commented Apr 8, 2023

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

cmdcolin added a commit to GMOD/jbrowse-components that referenced this issue Apr 8, 2023
@dperini dperini closed this as completed in 56192cc Apr 9, 2023
@dperini
Copy link
Owner

dperini commented Apr 9, 2023

@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 ?

@dperini dperini reopened this Apr 9, 2023
dperini added a commit that referenced this issue Apr 9, 2023
@cmdcolin
Copy link
Author

cmdcolin commented Apr 9, 2023

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

@cmdcolin
Copy link
Author

can probably close now

@cmdcolin
Copy link
Author

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

@janpe
Copy link

janpe commented Apr 12, 2023

@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
Copy link
Author

cmdcolin commented Apr 12, 2023

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

@jahumes
Copy link

jahumes commented Apr 12, 2023

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
Copy link
Owner

dperini commented Apr 12, 2023

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 dperini reopened this Apr 12, 2023
@dperini
Copy link
Owner

dperini commented Apr 12, 2023

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

@cmdcolin
Copy link
Author

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

@dperini dperini closed this as completed Apr 13, 2023
@jahumes
Copy link

jahumes commented Apr 13, 2023

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

@sagar1111212121
Copy link

sagar1111212121 commented Jul 3, 2023

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants