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

[Listbox] Right-hand side of 'instanceof' is not callable (SSR) #3471

Closed
halljus opened this issue Sep 10, 2024 · 4 comments · Fixed by #3494
Closed

[Listbox] Right-hand side of 'instanceof' is not callable (SSR) #3471

halljus opened this issue Sep 10, 2024 · 4 comments · Fixed by #3494
Assignees

Comments

@halljus
Copy link

halljus commented Sep 10, 2024

What package within Headless UI are you using?

@headlessui/react

What version of that package are you using?

v2.1.6 (but the problem can be reproduced beginning at v2.0.0)

What browser are you using?

N/A

Reproduction URL

Stackblitz: Vite SSR
Stackblitz: Remix
Stackblitz: Next.js

Describe your issue

When server rendering the Listbox component under certain circumstances, we get the following error:

TypeError: Right-hand side of 'instanceof' is not callable
    at Object.eval (file:///home/projects/vitejs-vite-cncj38/node_modules/@headlessui/react/dist/internal/floating.js:23:1298)
    at JSON.stringify (<anonymous>)
    at Module.Re (file:///home/projects/vitejs-vite-cncj38/node_modules/@headlessui/react/dist/internal/floating.js:23:1245)
    at Ct (file:///home/projects/vitejs-vite-cncj38/node_modules/@headlessui/react/dist/components/listbox/listbox.js:21:15764)
    at renderWithHooks (file:///home/projects/vitejs-vite-cncj38/node_modules/react-dom/cjs/react-dom-server-legacy.node.development.js#cjs:5662:16)
    at renderForwardRef (file:///home/projects/vitejs-vite-cncj38/node_modules/react-dom/cjs/react-dom-server-legacy.node.development.js#cjs:5857:18)
    at renderElement (file:///home/projects/vitejs-vite-cncj38/node_modules/react-dom/cjs/react-dom-server-legacy.node.development.js#cjs:6020:11)
    at renderNodeDestructiveImpl (file:///home/projects/vitejs-vite-cncj38/node_modules/react-dom/cjs/react-dom-server-legacy.node.development.js#cjs:6119:11)
    at renderNodeDestructive (file:///home/projects/vitejs-vite-cncj38/node_modules/react-dom/cjs/react-dom-server-legacy.node.development.js#cjs:6091:14)
    at renderNode (file:///home/projects/vitejs-vite-cncj38/node_modules/react-dom/cjs/react-dom-server-legacy.node.development.js#cjs:6274:12)

As you can see by the provided reproduction URLs, we can reproduce this across various SSR implementations. If you SSR the Listbox in an isolated environment, everything seems to be fine. But there are certain circumstances that cause this error and one we've been able to nail down is when it's in a project that also uses AG Grid. This will cause Listbox to error consistently even if it's rendered on an unrelated route (that is, a separate route from where we render the grid).

Your distributed files are minified, which makes it harder to pin down the error, but after manually prettify-ing @headlessui/react/dist/internal/floating.js, it appears the error comes from this statement on line 140:

let stablePlacement = useMemo(
    () => placement,
    [
      JSON.stringify(
        placement,
        typeof HTMLElement !== 'undefined'
          ? (_, v) => {
              if (v instanceof HTMLElement) { // <-- this throws
                return v.outerHTML
              }
              return v
            }
          : undefined
      ),
    ]
  )

In a project with just the Listbox, typeof HTMLElement is undefined, so this line is not executed (due to the ternary condition not being met). However, in a project with AG Grid, typeof HTMLElement is actually object and appears to be an empty object {} which is not a valid right-hand operand for instanceof.

This particular case is probably AG Grid doing something it shouldn't, but it wouldn't hurt to be more defensive here. Perhaps a try-catch and return v if it throws, or maybe there's some reliable way to ensure HTMLElement will be a valid instanceof operand, I'm not sure.

Thoughts?

@madhavisolanki
Copy link

I am facing the same issue

@halljus
Copy link
Author

halljus commented Sep 24, 2024

Can we get your thoughts on this, @RobinMalfait? Happy to PR the try-catch if it helps, but not sure if that's the way you want to handle it. Thanks. 🙏

@RobinMalfait RobinMalfait self-assigned this Sep 26, 2024
RobinMalfait added a commit that referenced this issue Sep 27, 2024
…3494)

This PR fixes an issue where in some environments where `HTMLElement` is
not
available (on the server) and AG Grid is used, we crashed.

This happens because the `HTMLElement` is polyfilled to an empty object.
This means that the `typeof HTMLElement !== 'undefined'` check passed,
but the `v instanceof HTMLElement` translated to `v instanceof {}` which
is invalid and resulted in a crash...

This PR solves it by checking for exactly what we need, in this case
whether the `outerHTML` property is available.

Alternatively, we could use `return v?.outerHTML ?? v`, but not sure if
that's always safe to do.

Fixes: #3471
@RobinMalfait
Copy link
Member

Hey! This is 100% an AG Grid related bug and not an Headless UI bug. That said, we can simplify our code a bit to also get rid of the bug so we can do that in the meantime.

This should be fixed by #3494, and will be available in the next release.

You can already try it using:

  • npm install @headlessui/react@insiders.

@halljus
Copy link
Author

halljus commented Sep 27, 2024

Confirmed the refactored version no longer crashes. Thanks for doing that, even though it's definitely an AG Grid faux pas. 🙏

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.

3 participants