cee-chen / object-fit-polyfill

A Javascript polyfill for browsers that don't support the object-fit CSS property.
ISC License
496 stars 93 forks source link

Incompatible with server side rendering #30

Closed Eschon closed 7 years ago

Eschon commented 7 years ago

I tried to use this polyfill in a react app that is rendered on the server and it fails because it can't access the document.

ReferenceError: document is not defined
    at /##########/node_modules/objectFitPolyfill/dist/objectFitPolyfill.min.js:1:105
    at Object.<anonymous> (/##########/node_modules/objectFitPolyfill/dist/objectFitPolyfill.min.js:1:2863)
    at Module._compile (module.js:569:30)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)
    at Function.Module._load (module.js:458:3)
    at Module.require (module.js:513:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/##########/server.js:1757:18)

Right now as a workaround I require it in the componentDidMount method of the component where I use it. But this is a bit cumbersome especially if I need it in multiple components.

Trevomatic commented 7 years ago

@Eschon @constancecchen I have a fix for server side rendering, it's not perfect, but if we add

  // If this is being rendered on the server, return
  if (typeof window === 'undefined') {
      return;
  }

I have tested it and it works fine with React.

I-NOZex commented 7 years ago

@Trevomatic I've used your branch has npm source, but still not working... (needs to update the *.min.js files too)

Eschon commented 7 years ago

I have just tried the fix and it works for me. Thanks @Trevomatic for taking the time to make a PR.

@constancecchen When do you plan to release a new Version that includes this fix?

cee-chen commented 7 years ago

Sorry for the long delay, just been super busy. I should've just published a new npm, bower, and git version that includes this fix.