-
Notifications
You must be signed in to change notification settings - Fork 13
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
Next.js build error: Window is not defined #228
Comments
@mikeriley131 Wow, that's a new one. Thanks for reporting. I need to play with it a little bit. I think it's because Next is experimenting with the server-client rendering and changing related things quite often. Did you try it with Next 14? |
Not yet. I'll give that a try and report back. |
I'm on a Next.js 14 project here, all up to date, this error is indeed happening still on 14. |
Thanks guys. I have no idea how to fix that and why Next treats the browser field in the package.json as a server entry point https://github.com/kkomelin/isomorphic-dompurify/blob/master/package.json#L30 |
Having similar issues. Uncaught ReferenceError: window is not defined at (webpack-internal:///(middleware)/../../node_modules/isomorphic-dompurify/browser.js:1) |
Can someone report this issue to Next.js developers? Interested to know what they think. |
As @BohdanYavorskyi suggested here I'm going to try adding |
I've released a new version with a fix for this bug. Please check it out and let me know if it works for you https://github.com/kkomelin/isomorphic-dompurify/releases/tag/v1.12.0 |
Hey. Thanks for your work! v.1.12 did not fix the issue for me, but don't stress yourself about it :) |
Thanks @alexmilde for your feedback. I didn't know that it's an issue for Svelte Kit as well. |
@kkomelin hey, do you have any ideas how this can be fixed? This issues blocks the upgrade on one of my projects :) |
@BohdanYavorskyi No, I don't, unfortunately. Next's innovations kind of turn things upside down. |
@kkomelin check this link, I guess same should be done for SSR support vercel/next.js#46893 But looks like you have the same in the package :( Perhaps this can be rewritten differently? |
@BohdanYavorskyi what exactly should I look at by the link you sent? Can you give me a link to the particular comment you meant?
Please go ahead and provide a pull-request. |
I came across this issue with v1.12.0 using SvelteKit deployed on Vercel Edge Functions (Vercel's custom runtime built on V8) This only happens in Vercel Edge Functions for me—deploying locally or to Vercel Serverless Functions (which uses Node as a runtime) works perfectly. Not quite the original issue but probably related. Error points to this line in browser.js as mentioned, so it does seem like browser.js is being used as an entrypoint incorrectly. module.exports = window.DOMPurify || (window.DOMPurify = require('dompurify').default || require('dompurify')); |
Thanks @silasabbott. I agree with your conclusion. Please comment and vote for this issue vercel/next.js#58142 |
For now, I'm going to rollback my attempt to fix this issue through this repo because it didn't work. |
@kkomelin hi, please replace this condition
with proper one
I tried to debug the old variant and got this on nextjs server :) |
@kkomelin can confirm it's working with the condition I sent (saw |
Are you sure that no one can define |
yes, all libraries check And I could make it work locally with just this condition |
@kkomelin can you release a new version ASAP? I can check it in old pages router and newest app router |
@BohdanYavorskyi I understand it's probably a cultural difference but I don't like your language like "proper one" or "can you release ASAP?". It's not my job and you're not my client to expect that. It's OpenSource. I asked you for a pull request, you didn't provide it but asked me to do it asap. Anyway, I'm still not convinced that What if I set Above we discussed the fact that Vercel mistakenly uses browser field value of package.json on the server. In what way will your suggested solution fix that issue? |
agree with the first sentence. My bad about second part nothing prevent us to set the check I mentioned as You can even see it in So I guess it should work fine without issues |
Which some frameworks actually do, but it doesn't mean it's 100% correct.
Could you please provide examples?
Some more checks from Dompurify code on top of that condition:
Source: https://github.com/cure53/DOMPurify/blob/d7498e0546c0f67cf95437ae430ff2035582a689/src/purify.js#L90
Guessing is not enough when you're at risk of breaking many sites. |
@kkomelin should I open a PR? How can we proceed further? |
Thanks @BohdanYavorskyi . I will take a look at your PR. |
Happy to announce that we've just released 2.0.0 where we switched from It won't fix the issue related to misusing the package.json Please test and let me know about the results. |
According to the error description, I can say that Next is trying to use the |
Finally had a chance to upgrade and test. Still getting "window is not defined" error in the Next.js logs, however it no longer prevents the page from rendering.
|
- Tried DOMPurify (isomorphic-dompurify) and stumbled that nextjs hydration has error message `window` undefined or other errors. kkomelin/isomorphic-dompurify#228 - Thus still use `dangerouslySetInnerHTML` and do sanitize on server side. This requires something like a directus hook extension: https://github.com/licitdev/directus-extension-sanitize-html There are some issues upstream, so I made my fork version and install it locally (not pushed online yet): - Manually build locally and copy `package.json` and `dist` folder to `directus/extensions/directus-extension-sanitize-html` volume. - The env variables must be sync with directus collection settings, current the collection (e.g., Facts) sanitizes ALL string fields. - Memo some interface setting of WYSIWYG UI, I do this setting for every field, didn't find a way to reuse.
isomorphic-dompurify version - 1.9.0
Node version - 18.18.0
Next.js version - 13.5.4
React version - 18.2.0
I am importing my company's React component library that makes use of Isomorphic DOMPurify.
No issues when using it in a client-side app.
However, when running
yarn build
in my Next.js project I am gettingReferenceError: window is not defined
and the path in the logs points to the following line of code:var ti=window.DOMPurify||(window.DOMPurify=Vl().default||Vl());
Am I doing something incorrectly?
Here's a repo for the Next.js app - https://github.com/mikeriley131/next-idp-test
And here's a repo for the component library - https://github.com/mikeriley131/pyrographic-react-component-library
The component library is "npm link"ed to the next app.
When I add the Button component from the library to the Next.js app, it throws the following error:
The text was updated successfully, but these errors were encountered: