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

SyntaxError: 'slot):not([inert]' is not a valid selector #986

Closed
josephvu519 opened this issue May 8, 2023 · 14 comments · Fixed by #990
Closed

SyntaxError: 'slot):not([inert]' is not a valid selector #986

josephvu519 opened this issue May 8, 2023 · 14 comments · Fixed by #990

Comments

@josephvu519
Copy link

josephvu519 commented May 8, 2023

I'm trying to create a modal component and unit test it. The tests did pass before, but now I'm getting an issue with the tests (Component still works fine) with the error stated in the title.

Component:

 export const Modal = () => {
     return (
         <FocusTrap>
             <div id="modal-dialog" className="modal">
                 <button>Ok</button>
                 <button>Cancel</button>
             </div>
         </FocusTrap>
     );
 };
 
 export default Modal;

Test File:

it('component should exist', () => {
         const component = render(<Modal />);
         expect(component).toBeTruthy();
});

Where the error is also pointing at render.
If I remove the div and only include a singular button, the test runs through.

If it helps, I found this syntax here: https://github.com/focus-trap/tabbable/blob/master/src/index.js#L12

@stefcameron
Copy link
Member

@josephvu519 Thanks for reaching out about it. This is directly related to focus-trap/tabbable#995 which was published just recently in tabbable v6.1.2.

I was hoping I could get a new feature on focus-trap (also a dependency of focus-trap-react) into the code before cascade-publishing focus-trap and focus-trap-react with the new tabbable, but I guess I might have to push it through without that other thing if you're seeing it now as well.

The temporary fix was simple for tabbable: https://github.com/focus-trap/tabbable/blob/master/package.json#L95

In the meantime, you can probably get around this by using the same concept, either exactly as-is, or using it to override the upstream version of tabbable you're using to pin it to 6.1.2 until I publish an updated focus-trap-react that use it.

See the original tabbable issue for more background if you want: focus-trap/tabbable#982

@kylemcd
Copy link

kylemcd commented May 10, 2023

@stefcameron Do you have a timeframe of when the new version of focus-trap-react will be published in order to fix the dependency issue? Absolutely no rush, I know you're probably juggling a million things! Just want to be able to make a decision about the patch we should make internally to get our testing suite green again. Thanks!

@stefcameron
Copy link
Member

@kylemcd Today was my target day, actually! I will post here once it's published. 🙂

stefcameron added a commit that referenced this issue May 10, 2023
Fixes #986

Note that tabbable was already bumped separately earlier today by
Dependabot in #989.
stefcameron added a commit that referenced this issue May 10, 2023
…990)

Fixes #986

Note that tabbable was already bumped separately earlier today by
Dependabot in #989.
@stefcameron
Copy link
Member

@kylemcd Published in v10.1.2 not long ago!

@OMManko
Copy link

OMManko commented May 11, 2023

hi @stefcameron. Thanks for your work! I updated my focus-trap-react to 10.1.2 (cleaned cache, reinstalled node modules etc.) and unfortunately this issue is still reproduced, the tests are failing with the error mentioned above :(

@stefcameron
Copy link
Member

@OMManko My pleasure! Hmm, that's curious. What version of nwsapi was installed in your local node_modules dir? (Should be 2.2.2), and what version of tabbable was installed (should be 6.1.2)?

@OMManko
Copy link

OMManko commented May 11, 2023

@OMManko My pleasure! Hmm, that's curious. What version of nwsapi was installed in your local node_modules dir? (Should be 2.2.2), and what version of tabbable was installed (should be 6.1.2)?

Yes, 2.2.2 and 6.1.2

@kylemcd
Copy link

kylemcd commented May 11, 2023

Hey hey, thanks for pushing the update last night! I went ahead and installed the latest version and I'm in the same boat as @OMManko where I'm seeing the same issue when running jest.

Seeing it happening with these versions installed:

nwsapi 2.2.2
tabbable 6.1.2
focus-trap-react 10.1.2
focus-trap 7.4.1

We're currently on jest 29.5.0 and also utilizing @testing-library/react 9.2.0

Let me know if there's any more information I can provide that could be helpful!

@stefcameron
Copy link
Member

@kylemcd @OMManko This really is an issue with nwsapi, not tabbable, not focus-trap, and not focus-trap-react.

When I do this, all tests pass:

$ git clone git@github.com:focus-trap/focus-trap-react.git
$ npm install
$ npm test

That's a clean node_modules installation resulting those same versions you quoted:

nwsapi 2.2.2
tabbable 6.1.2
focus-trap-react 10.1.2
focus-trap 7.4.1

The nwsapi author is trying to fix the problem: dperini/nwsapi#83 (comment) Probably the best thing you could do is help him out with his outstanding regex question. I haven't had a chance to really check it out.

As for your immediate problem, one thing I thought to ask is whether you're using yarn or npm to install your dependencies, and if yarn v1.x, it probably doesn't support the overrides field in package.json so maybe (and perhaps regardless of the npm vs yarn idea) there's still another version of nwsapi being installed in a sub-node_modules directory used with something else.

Also, RTL's latest version is 14.0.0 -- 9.2.0 you're using is very, very old. It could be another source of problems with the downstream dependencies.

@ApacheEx
Copy link

ApacheEx commented May 11, 2023

Hey guys, as workaround to this issue (if the latest version doesn't help), you can add the following to your package.json

"resolutions": {
  "nwsapi": "2.2.2"
}

@stefcameron
Copy link
Member

Right! ☝️ I've done the most I can do from a tabbable/focus-trap/focus-trap-react perspective to try to mitigate the issue. But it may be that the override still needs to be added to your project's package.json because of other dependencies that are still pulling in a bad version of nwsapi.

@kylemcd
Copy link

kylemcd commented May 11, 2023

Sounds good, the override works for now. Seems like there is a lot more at play here with other packages (potentially?) also relying on nwsapi and it resolving to the wrong version in our package-lock.json. Interesting thing is the only version of nwsapi that shows up in our package-lock stems from focus-trap-react which is set to 2.2.2 but for some reason it still resolves to 2.2.4. I'm at the outskirts of my knowledge when it comes to package dependency resolution so not sure exactly what is causing that.

Either way, thanks for the tip on the override!

@OMManko
Copy link

OMManko commented May 12, 2023

@stefcameron @ApacheEx thank you so much, guys!

@stefcameron
Copy link
Member

@OMManko You're welcome! 😀

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

Successfully merging a pull request may close this issue.

5 participants