dialog-polyfill icon indicating copy to clipboard operation
dialog-polyfill copied to clipboard

No support for imports from isomorphic scripts

Open jonathandewitt-dev opened this issue 3 years ago • 3 comments

I'm using this polyfill in basically every project now, you guys are life savers, so thanks for that!

I tend to use a lot of universal JavaScript frameworks, especially Next.js. One of the benefits of doing so is server-side rendering in Node in combination with client-side routing - in other words, isomorphic templating. However, there's a lot of things to be careful about when you run the same code on client and server, like when trying to access window. As you're probably aware, Node is very particular about references to window.

Hence, the references to window within this package's code cause our builds to fail. Without ever making a reference to dialogPolyfill, this static import causes an error by itself:

import dialogPolyfill from 'dialog-polyfill'

My only work-around so far has been a dynamic import, only after I'm sure the script is in a client-side context.

if (typeof window !== 'undefined') {
  const dialogPolyfill = await import('dialog-polyfill')
}

... but that isn't ideal for me, especially considering how difficult it can be to diagnose this problem.

I would much rather keep a consistent import style, then handle calling it on the client side myself. For example, in a typical Next component:

import { useRef, useEffect } from 'react'
import dialogPolyfill from 'dialog-polyfill'

const ReactDialog = () => {
  const dialogRef = useRef(null)

  useEffect(() => {
    dialogPolyfill.registerDialog(dialogRef.current)
  }, [])

  return <dialog ref={dialogRef}>Hello World</dialog>
}

Since there is only a benefit client-side, I might expect a clear error to be thrown if I inadvertently use the polyfill on the server side, but that level of error handling can help the consumer of the package to easily troubleshoot and solve the problem. Otherwise, the entire build fails with a less descriptive error complaining that window is undefined.

jonathandewitt-dev avatar Mar 16 '22 00:03 jonathandewitt-dev

To address this issue, I've created a pull request myself, for your review: https://github.com/GoogleChrome/dialog-polyfill/pull/231

jonathandewitt-dev avatar Mar 16 '22 01:03 jonathandewitt-dev

FWIW I published dialog-polyfill-universal for SSR compatibility.

targos avatar Jun 27 '22 11:06 targos

Just wanted to add that I have experienced this exact issue. If you would consider reviewing the PR raised by @jonathandewitt-dev that would be incredible please ❤️

jamieomaguire avatar Sep 19 '23 16:09 jamieomaguire