ayrton / react-key-handler

React component to handle keyboard events :key:
http://ayrton.be/react-key-handler
388 stars 29 forks source link

v1.1.0 window is not defined #149

Closed ayrton closed 6 years ago

ayrton commented 6 years ago
ReferenceError: window is not defined
    at Object.<anonymous> (/app/node_modules/react-key-handler/dist/index.js:1:258)
    at Module._compile (module.js:624:30)
    at Object.Module._extensions..js (module.js:635:10)
    at Module.load (module.js:545:32)
    at tryModuleLoad (module.js:508:12)
    at Function.Module._load (module.js:500:3)
    at Module.require (module.js:568:17)
    at require (internal/module.js:11:18)
    at Object.zkQZ (/app/public/assets/webpack:/external "react-key-handler":1:1)
    at __webpack_require__ (/app/public/assets/webpack:/webpack/bootstrap cfaabd2047db963ff127:25:1)
    at Object.N7P3 (/app/public/assets/webpack:/frontend/ph/components/App.js:3:1)
    at __webpack_require__ (/app/public/assets/webpack:/webpack/bootstrap cfaabd2047db963ff127:25:1)
    at Object.lYkL (/app/public/assets/webpack:/frontend/server/routes/ssr.js:1:1)
    at __webpack_require__ (/app/public/assets/webpack:/webpack/bootstrap cfaabd2047db963ff127:25:1)
    at Object.TDg2 (/app/public/assets/webpack:/frontend/server/ssr.js:17:1)
    at __webpack_require__ (/app/public/assets/webpack:/webpack/bootstrap cfaabd2047db963ff127:25:1)
    at Object.0 (/app/public/assets/server.js:2373:18)
    at __webpack_require__ (/app/public/assets/webpack:/webpack/bootstrap cfaabd2047db963ff127:25:1)
    at /app/public/assets/webpack:/webpack/bootstrap cfaabd2047db963ff127:90:1
    at Object.<anonymous> (/app/public/assets/server.js:95:10)
    at Module._compile (module.js:624:30)
    at Object.Module._extensions..js (module.js:635:10)
    at Module.load (module.js:545:32)
    at tryModuleLoad (module.js:508:12)
    at Function.Module._load (module.js:500:3)
    at Function.Module.runMain (module.js:665:10)
    at startup (bootstrap_node.js:187:16)
    at bootstrap_node.js:607:3
gforge commented 6 years ago

? is this from the Flow updates? In what environment?

ayrton commented 6 years ago

This is new since the latest release. This happens during feature/integrations tests when SSR'ing React

gforge commented 6 years ago

Is this a canusedom issue?

ayrton commented 6 years ago

My first thought was this might be a npm build issue (with the upgraded webpack module)

gforge commented 6 years ago

Try using ExecutionEnvironment.canUseEventListeners instead of the ExecutionEnvironment.canUseDOM, I've never used ssr so I don't know how to even set it up

gforge commented 6 years ago

Really weird that this even can happen, https://github.com/JedWatson/exenv/blob/master/index.js#L11 is not that fancy - if the window isn'tthere then it shouoldn't be called. Have you checked the rollup version?

ayrton commented 6 years ago

This isn't a exenv afaik, just an issue with the current build step. I haven't checked the rollup version, can you link me to it, if we are going to switch anyway we might just bypass this issue alltogether

gforge commented 6 years ago

You can find it here: https://github.com/gforge/react-key-handler/tree/rollup

just clone and checkout origin/rollup

ayrton commented 6 years ago

@gforge super helpful, I'm removing the webpack dependency in #150 and replacing it with parcel + rollup. Will release a 1.2.0 rc once finished, and when confirmed it's fully working we'll be able to move on to your next PR

gforge commented 6 years ago

Awesome, I've merged with your upstream changes. I've heard of parcel but never used it - seems pretty neat. The packaging tools are way too complicated and if parcel delivers it seems great.

ayrton commented 6 years ago

I'm hoping to find the time tomorrow to release + test this out on the Product Hunt main app (via a release candidate). If all seems well I'll release an official version, will keep you up to date.

Parcel is great as in that it's a no configuration app bundler, I like it.

gforge commented 6 years ago

Did you manage to track down the bug?

ayrton commented 6 years ago

Having gotten the chance to test out the Rollup version but I am confident this will have addressed this bug. Been travelling, I will be able to release this this week, sorry for the holdup

On 19 Aug 2018, at 20:42, Max Gordon notifications@github.com wrote:

Did you manage to track down the bug?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

MathieuDebit commented 6 years ago

Hello 👋 I also have this issue since 1.1.0 in an Electron app that use a package with react-key-handler as a dependency.

Working fine with 1.0.1, and waiting for the next update. Thanks 🙏

ayrton commented 6 years ago

Fixed in 1.2.0-beta.2