oddbird / popover-polyfill

Polyfills the HTML popover attribute and showPopover/hidePopover/togglePopover methods onto HTMLElement, as well as the popovertarget and popovertargetaction attributes on <button> elements.
BSD 3-Clause "New" or "Revised" License
247 stars 14 forks source link

Library is incompatible with Server-Side Rendering (SSR) #192

Closed fzembow closed 5 months ago

fzembow commented 5 months ago

I'm using this library with server side rendering (SvelteKit in my case, but it shouldn't matter) and when importing the library on the server side, I am getting a Node error window is not defined

It looks like there is a top-level reference to window since support for CSS layers was added a few months back (https://github.com/oddbird/popover-polyfill/pull/178/files#diff-69e41edf30fc514f48f42b118e1a634d01e739c8c5b5981a35f545cdf645903eR37)

var hasLayerSupport = typeof window.CSSLayerBlockRule === "function";

Could this either be checked at usage time instead of at the top level like this? Or check for window being defined before accessing properties on it?

Sorry, I would submit a PR myself but I have a three week old child and am totally exhausted for picking up new tasks at the moment :)

Thank you!! I am happy to use modern APIs when possible and so this polyfill makes that possible

jgerigmeyer commented 5 months ago

Included in v0.4.1 release.

fzembow commented 5 months ago

Thank you @jgerigmeyer, works great now :)


For any future visitors to this issue who might be struggling with SSR:

Make sure you are using the /fn import and checking for support before applying the polyfill

    import { isSupported, apply } from '@oddbird/popover-polyfill/fn';

    // ...

    if (typeof window !== 'undefined' && !isSupported) {
        apply();
    }

Importing the main import will automatically apply it, which happens on the server as well and will not work if window / document are not present.

jgerigmeyer commented 5 months ago

Hm, I wonder if isSupported should check for that. @keithamus What do you think?

keithamus commented 5 months ago

Sounds like a reasonable thing to do. No point applying if some base classes don’t exist.

jgerigmeyer commented 4 months ago

@fzembow Starting in v0.4.3 the polyfill will check for typeof window !== 'undefined' before running, so you should be able to use the default build in SSR contexts.