Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Maybe we should check for != null instead of typeof "undefined" #16

Open
rpearce opened this issue Mar 3, 2020 · 2 comments
Open

Maybe we should check for != null instead of typeof "undefined" #16

rpearce opened this issue Mar 3, 2020 · 2 comments

Comments

@rpearce
Copy link

rpearce commented Mar 3, 2020

I'm having an annoying issue where this all works locally but running with js-dom in travisCI, it can't find document (it's null), so this package breaks the tests, unfortunately.

Error: Uncaught [TypeError: Cannot read property 'scrollingElement' of null]

at calcScrollableElements (/home/travis/build/rpearce/react-medium-image-zoom/node_modules/focus-options-polyfill/index.js:41:18)

https://github.com/calvellido/focus-options-polyfill/blob/master/index.js#L3-L9

It might be a good idea to change those to be != null instead of typeof ... === "undefined". Using != null covers both undefined and null use cases. Another option would be simply to use the ! operator in front of each one.

What do you think?

@jonaskuske
Copy link

But I'm pretty sure that both the ! operator and != null will throw if document doesn't exist at all, e.g. in Node... So both typeof and one of these would be necessary..

Anyhow, why is JSDOM setting document to null in your setup? Isn't emulating document (and the rest of the DOM) basically the whole point there? 😅

@rpearce
Copy link
Author

rpearce commented Apr 27, 2020

It seems I misspoke about js-dom; it throws an error while building the JS output in TravisCI, for there is no browser environment at the time the code is executed:
https://travis-ci.org/github/rpearce/react-medium-image-zoom/builds/680096939

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants