GoogleChrome / dialog-polyfill

Polyfill for the HTML dialog element
BSD 3-Clause "New" or "Revised" License
2.45k stars 245 forks source link

No support for imports from isomorphic scripts #230

Open jonathandewitt-dev opened 2 years ago

jonathandewitt-dev commented 2 years ago

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 commented 2 years ago

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

targos commented 2 years ago

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

jamieomaguire commented 1 year ago

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 ❤️