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

webpack build error: document is not defined #14

Closed
jeremygoccc opened this issue Feb 9, 2019 · 22 comments · Fixed by #22
Closed

webpack build error: document is not defined #14

jeremygoccc opened this issue Feb 9, 2019 · 22 comments · Fixed by #22

Comments

@jeremygoccc
Copy link

I am planning to add dark mode for my website and import this package, it works in development but when building it throws an error: document is not defined. I tried to pass the optioalConfigObject but still get the error.

donavon added a commit that referenced this issue Feb 9, 2019
possibke solution for #14
donavon added a commit that referenced this issue Feb 9, 2019
* fix for document is not defined

possibke solution for #14

* bump version to 2.1.2
@donavon
Copy link
Owner

donavon commented Feb 9, 2019

Do me a favor. Try this out with version 2.1.2. I have a small fix that used global.document vs just document.

@jeremygoccc
Copy link
Author

Now the error is: Cannot read property 'body' of undefined.

@wKovacs64
Copy link
Contributor

This is happening in SSR/SSG tools because document isn't available in Node (where this code runs at build-time). Some options:

  • add a typeof document !== 'undefined' check around the element assignment in defaultConfig (or wrap the assignment in a try-catch and let the error fall on the floor, etc.)
  • change element from an actual element to a selector string on which you later call document.querySelector once it's safe to do so (setDOMClass maybe, since that only gets called from within the useEffect?)

I'd probably lean towards the first approach as it doesn't change your public API. Maybe there are better solutions?

Note: this will be a problem in use-persisted-state as well since localStorage is not available either.

@donavon
Copy link
Owner

donavon commented Feb 9, 2019

@wKovacs64,

looking to do this in useDarkMode

const noop = () => {};

const defaultElement = (global.document && global.document.body) || {
  classList: {
    add: noop,
    remove: noop,
  },
};

const defaultConfig = {
  classNameDark: defaultClassNameDark,
  classNameLight: defaultClassNameLight,
  element: defaultElement,
};

basically, this will provide a mock element with a classList object with add and remove methods if global.document.body doesn't exist.

Overkill? Thoughts? I haven't worked with SSR at all.

But before I do, I have to fix use-persisted-state to support SSR as well.

@wKovacs64
Copy link
Contributor

I suggested that type of approach in a similar project a while back but now I'm not sure it really matters. Whatever is cleanest/easiest to maintain is probably fine since all we're doing is faking it out to make it build.

Hmm, the no-flash solution might be in jeopardy here. 🤔

@donavon
Copy link
Owner

donavon commented Feb 9, 2019

the no-flash code only runs client-side as it's included in index.html, so no issues there.

@wKovacs64
Copy link
Contributor

I was thinking it might get evaluated during build but it shouldn't. 👍

(For a lot of these SSR/SSG tools, there isn't an index.html to modify. It's generated, so we just pass in some custom JSX.)

@donavon
Copy link
Owner

donavon commented Feb 10, 2019

@fxbabys

I've create an alpha for you to test against. Test it by installing:

npm i use-dark-mode@next

I tested it myself with an SSR test app, but would like you to verify before I put the code into the @latest version.

@jeremygoccc
Copy link
Author

jeremygoccc commented Feb 10, 2019

It works now, and I can also pass my custom config. Thank you !

@donavon
Copy link
Owner

donavon commented Feb 10, 2019

nice! I'll create a real @latest version 2.2.0 and publish it tomorrow. Thx fo reporting this and working with me to get it tested.

@allcontributors[bot] please add @fxbabys for userTesting and bug

@allcontributors
Copy link
Contributor

@donavon

I've put up a pull request to add @fxbabys! 🎉

@donavon
Copy link
Owner

donavon commented Feb 10, 2019

@allcontributors[bot] please add ideas to @wKovacs64

@allcontributors
Copy link
Contributor

@donavon

I've put up a pull request to add @ideas! 🎉

@donavon
Copy link
Owner

donavon commented Feb 10, 2019

arrg. merge conflict. again...

@allcontributors[bot] please add ideas to @wKovacs64

@allcontributors
Copy link
Contributor

@donavon

I've put up a pull request to add @ideas! 🎉

@donavon
Copy link
Owner

donavon commented Feb 10, 2019

one more time...

@allcontributors[bot] please add ideas to @wKovacs64

@allcontributors
Copy link
Contributor

@donavon

I've put up a pull request to add @ideas! 🎉

@wKovacs64
Copy link
Contributor

I don't think that worked. 😄 (The bot, I mean. The changes in @next work!)

@donavon
Copy link
Owner

donavon commented Feb 10, 2019

yeah. the bot is a little finicky. i'll have to manually back it out and add ideas. 🤦‍♂️

@donavon donavon mentioned this issue Feb 10, 2019
@jakebolam
Copy link

Hey! Thanks for using the bot :)

I think it's expecting the syntax bot please add username for blah we're working on improving this, and auto fixing merge conflicts 😀

@donavon
Copy link
Owner

donavon commented Feb 10, 2019

@fxbabys

I pushed the SSR support into 2.2.1 (@latest). Can you please just verify that it still works for you.

Thx!

@jeremygoccc
Copy link
Author

Sure, and it still works. 😄

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

Successfully merging a pull request may close this issue.

4 participants