kkomelin / isomorphic-dompurify

Use DOMPurify on server and client in the same way
MIT License
425 stars 13 forks source link

Can't resolve 'canvas' on next.js serverless app #54

Open EnricoNapolitan opened 3 years ago

EnricoNapolitan commented 3 years ago

What's the problem

I built a next.js application with serveless option, but simply importing this library it throws this error:

Failed to compile.
ModuleNotFoundError: Module not found: Error: Can't resolve 'canvas' in '/.../node_modules/jsdom/lib/jsdom'
> Build error occurred

I already tried to add canvas library but this doesn't fix the problem and it throws another error.

Step to reproduce

Add to next.config.js:

module.exports = {
  target: 'serverless'
}

Run next build

Basic Info:

This issue could be related to #35 , but i don't use umijs.

eharkins commented 3 years ago

@EnricoNapolitan did you find a solution? I'm encountering something similar with Gatsby.

kkomelin commented 3 years ago

Thanks for your detailed bug report @EnricoNapolitan. Since many people experience this issue, I will try to find a solution myself. Linking it here just for reference https://github.com/kkomelin/isomorphic-dompurify/pull/36

NicholasEllul commented 3 years ago

I think I've seen this come up with JSDOM before. JSDOM marks canvas as an optional dependency https://github.com/jsdom/jsdom/blob/master/package.json#L58

kkomelin commented 3 years ago

Hi @NicholasEllul, Thanks for the info. I'd like to try to add Canvas as an optional dependency too and see if it fixes the issue.

kkomelin commented 3 years ago

I've reproduced the bug locally. Will try to find a solution.

kkomelin commented 3 years ago

My hypothesis that adding canvas as an optional dependency to isomporphic-dompurify didn't work out. I'm thinking about adding canvas as a normal dependency. What do you think about it, @NicholasEllul?

How would it affect our existing users? Should we release a major version with that change?

kkomelin commented 3 years ago

I've tried to add canvas as a normal dependency but it caused new errors on npm run build:

> next build

info  - Creating an optimized production build  
Failed to compile.

./node_modules/canvas/build/Release/canvas.node 1:0
Module parse failed: Unexpected character '' (1:0)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
(Source code omitted for this binary file)

> Build error occurred
Error: > Build failed because of webpack errors
    at [my path]/nextjs-dompurify/node_modules/next/dist/build/index.js:15:918
    at async [my path]/nextjs-dompurify/node_modules/next/dist/build/tracer.js:3:470
npm ERR! code 1
npm ERR! path [my path]/nextjs-dompurify
npm ERR! command failed
npm ERR! command sh -c next build

npm ERR! A complete log of this run can be found in:
npm ERR!     [my path]/.npm/_logs/2021-04-30T11_28_12_323Z-debug.log

Maybe it's my local Ubuntu issue only but we can't be sure that it won't happen with other OSes too.

NicholasEllul commented 3 years ago

Just tested on MacOS and building succeeded for me. However, I agree we cant be sure about other environments. There seems to be a reason JSDOM makes canvas optional despite some functionality requiring it

https://github.com/jsdom/jsdom/issues/2603

So if we make it mandatory, we may hit the same issues

kkomelin commented 3 years ago

Thanks @NicholasEllul . If adding canvas as an optional dependency to isomporphic-dompurify doesn't solve the problem and adding it as a normal dependency can potentially break things, I don't have other choices for now to just mention this issue in the documentation - that our project doesn't work for Next.js serverless setup and some other server-side environments. Unless you see any other options.

nnduc1994 commented 2 years ago

I have a more or less similar issue while trying to use isomorphic-dompurify with AWS cdk and esbuild, got following errors the 2 warnings something to do with esbuild, I assume

 > node_modules/jsdom/lib/jsdom/utils.js: error: Could not resolve "canvas" (mark it as external to exclude it from the bundle)
    160 │   const Canvas = require("canvas");
        ╵                          ~~~~~~~~

 > node_modules/jsdom/lib/jsdom/living/xhr/XMLHttpRequest-impl.js: warning: Indirect calls to "require" will not be bundled (surround with a try/catch to silence this warning)
    31 │ const syncWorkerFile = require.resolve ? require.resolve("./xhr-sync-worker.js") : null;
       ╵                        ~~~~~~~

 > node_modules/jsdom/lib/jsdom/living/xhr/XMLHttpRequest-impl.js: warning: "./xhr-sync-worker.js" should be marked as external for use with "require.resolve"
    31 │ const syncWorkerFile = require.resolve ? require.resolve("./xhr-sync-worker.js") : null;
zhengminhui commented 2 years ago

@nnduc1994 Hi, did you resolve this error, please?

DennisRosen commented 2 years ago

Just ran into this in Gatsby, any updates? Also receiving the same "Module parse failed..." error if I try to use this package with canvas installed. I'll switch to using sanitize-html for now instead.

DePasqualeOrg commented 2 years ago

I was also having this issue with SvelteKit using the Node adapter. Installing canvas solved the issue, but it would be nice if there were also a solution for serverless.

Dinkelborg commented 1 year ago

Still seeing this issue using gatsby and trying to use the isomorphic-dompurify in a template file

libbyberrie commented 1 year ago

Still seeing this issue using gatsby and trying to use the isomorphic-dompurify in a template file

Same for me here

Antonio-Laguna commented 1 year ago

Noting that this does not work (for me at least) on Next 13 not even on Serverless but normal out of the box Next app

kkomelin commented 1 year ago

@Antonio-Laguna

Noting that this does not work (for me at least) on Next 13 not even on Serverless but normal out of the box Next app

This one is new. Do you use the experimental app/ folder? Could you please give me more info?

Antonio-Laguna commented 1 year ago

@kkomelin yes that was on the experimental app/ folder using use client. It was just a quick test since I'm trying to build a library and we're leveraging that package on a component.

I'm happy to do more testing and even contribute to fixes if I know what could be going on

kkomelin commented 1 year ago

Thank you @Antonio-Laguna. I don't think there is much we can do here at the isomorhic-dompurify level because it's just a tiny wrapper around dompurify and jsdom, and the problem lays in the way jsdom communicates with canvas dependency. But if you have any ideas how we can fix it through this package, please let me know.

Antonio-Laguna commented 1 year ago

Is this tracked at JSDOM level somehow though?

This might be extremely simplistic but shouldn't dompurify leverage the DOM instead of JSDOM which is intended to be leveraged only at Node level?

kkomelin commented 1 year ago

@Antonio-Laguna

Usually the error message looks like this:

Failed to compile. ModuleNotFoundError: Module not found: Error: Can't resolve 'canvas' in '/.../node_modules/jsdom/lib/jsdom' Build error occurred

What's your error message?


As for the server vs client code, modern JS frameworks flirt a lot with the server (SSR, Server Components, etc) and Next is not an exception. Dompurify itself leverages DOM and isomorphic-dompurify utilizes JSDOM to make dompurify work on the server.

karlhorky commented 1 year ago

Solution

For usage of DOMPurify in Next.js without the wrapper package isomorphic-dompurify, here is a demo of a simple version:

Mainly the code is this (this is a Server Component - if you aren't using the app/ directory, then do the DOMPurify() call in getServerSideProps or an API Route):

import DOMPurify from "dompurify";
import { JSDOM } from "jsdom";

export default function Home() {
  return (
    <div
      dangerouslySetInnerHTML={{
        __html: DOMPurify(new JSDOM("<!DOCTYPE html>").window).sanitize(
          "<img src=x onerror=alert(1)//>"
        ),
      }}
    />
  );
}

There is also some required Next.js configuration for this, to resolve the webpack bundling errors:

next.config.js

/** @type {import("next").NextConfig} */
module.exports = {
  reactStrictMode: true,
+ webpack: (config) => {
+   config.externals = [...config.externals, "canvas", "jsdom"];
+   return config;
+ },
};
kkomelin commented 1 year ago

Thanks @karlhorky. As I already asked here https://github.com/cure53/DOMPurify/issues/400#issuecomment-1458624807, since isomorphic-dompurify does the same under the hood as you illustrated in the code example, I guess it will work just fine with the webpack configuration you suggested. Can you test it for me?

karlhorky commented 1 year ago

Should be fine, try modifying the CodeSandbox demo to use your library instead of dompurify:

https://codesandbox.io/p/sandbox/lingering-river-j5fpi7?file=%2Fapp%2Fpage.tsx

kkomelin commented 1 year ago

Should be fine, try modifying the CodeSandbox demo to use your library instead of dompurify: https://codesandbox.io/p/sandbox/lingering-river-j5fpi7?file=%2Fapp%2Fpage.tsx

Just confirmed. Your webpack fix works for isomorphic-dompurify as well.

Daggron commented 1 year ago

For usage of DOMPurify in Next.js without the wrapper package isomorphic-dompurify, here is a demo of a simple version:

Mainly the code is this (this is a Server Component - if you aren't using the app/ directory, then do the DOMPurify() call in getServerSideProps or an API Route):

import DOMPurify from "dompurify";
import { JSDOM } from "jsdom";

export default function Home() {
  return (
    <div
      dangerouslySetInnerHTML={{
        __html: DOMPurify(new JSDOM("<!DOCTYPE html>").window).sanitize(
          "<img src=x onerror=alert(1)//>"
        ),
      }}
    />
  );
}

There is also some required Next.js configuration for this, to resolve the webpack bundling errors:

next.config.js

/** @type {import("next").NextConfig} */
module.exports = {
  reactStrictMode: true,
+ webpack: (config) => {
+   config.externals = [...config.externals, "canvas", "jsdom"];
+   return config;
+ },
};

Holy Crap the next config fix worked for me 🥳

Daggron commented 1 year ago

Should be fine, try modifying the CodeSandbox demo to use your library instead of dompurify:

https://codesandbox.io/p/sandbox/lingering-river-j5fpi7?file=%2Fapp%2Fpage.tsx

@kkomelin @karlhorky Can you put more details into this why webpack changes fix this issue ?

karlhorky commented 1 year ago

Don't know too much about it, check out the details I know here:

abedshaaban commented 1 year ago

i still have this error when using react-pdf Basic Info:

    "@types/react-pdf": "^6.2.0",
    "next": "13.2.4",
    "react": "18.2.0",
    "react-dom": "18.2.0",
    "react-pdf": "^6.2.2"

Error Message

./node_modules/pdfjs-dist/build/pdf.js:10759:0
Module not found: Can't resolve 'canvas'

https://nextjs.org/docs/messages/module-not-found

Import trace for requested module:
./node_modules/react-pdf/dist/cjs/entry.js
./component/PDF/index.tsx

when I add "canvas"

Failed to compile
./node_modules/canvas/build/Release/canvas.node
Module parse failed: Unexpected character '�' (1:2)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
(Source code omitted for this binary file)

I still didn't update it to the next 13.3 since when I run yarn dev on windows, it crashes and throws errors

mephobic commented 1 year ago

@abedshaaban I found a way to resolve this when using react-pdf in Next Js 13 (app directory)

  1. The best way In your app directory, whatever/page.tsx have this
    "use client"
    import { pdfjs } from "react-pdf";
    pdfjs.GlobalWorkerOptions.workerSrc = 'https://cdnjs.cloudflare.com/ajax/libs/pdf.js/2.16.105/pdf.worker.min.js';

In your next.config.js have this

/** @type {import('next').NextConfig} */
const nextConfig = {
  experimental: {
    appDir: true,
  },
  future: { webpack5: true },
  webpack: (config, { buildId, dev, isServer, defaultLoaders, webpack }) => {
      config.resolve.alias.canvas = false
      config.resolve.alias.encoding = false
      return config
  }
}

module.exports = nextConfig

Let me know if it doesn't work. Happy to help out. This took me waaaay too long to figure out. Hopefully it will help someone get there faster! :)

abedshaaban commented 1 year ago

@mephobic i had a solution similar to that webpack: (config) => { config.externals.push({ sharp: 'commonjs sharp', canvas: 'commonjs canvas' }) return config

However, when I try yours, I get an error when I refresh the page with the error of Error Decoder, In the minified production build of React, we avoid sending down full error messages in order to reduce the number of bytes sent over the wire.

what do you think the problem is ?

mephobic commented 1 year ago

@mephobic i had a solution similar to that webpack: (config) => { config.externals.push({ sharp: 'commonjs sharp', canvas: 'commonjs canvas' }) return config

However, when I try yours, I get an error when I refresh the page with the error of Error Decoder, In the minified production build of React, we avoid sending down full error messages in order to reduce the number of bytes sent over the wire.

what do you think the problem is ?

It's difficult to debug without seeing what your error is. Can you try a. Running the code on your local and get the full error (because a local development code doesn't minify) or b. Run this - const nextConfig = { // ... productionBrowserSourceMaps: true, // ... };

magicink commented 1 year ago

I don't know if this is directly related as I don't deploy to a lambda, but I thought it might be worth a mention. I saw this exact same error when I upgraded from 0.18.0 to 1.6.0 on my NextJS 13 stack. I had to do two things. I had a direct dependency of DOMPurify 3.0.3. Removing that dependency entirely and downgrading to 0.18.0 resolved the issue for me.

In case it matters, my yarn.lock now references DOMPurify 2.4.x

hrc7505 commented 1 year ago

For usage of DOMPurify in Next.js without the wrapper package isomorphic-dompurify, here is a demo of a simple version:

Mainly the code is this (this is a Server Component - if you aren't using the app/ directory, then do the DOMPurify() call in getServerSideProps or an API Route):

import DOMPurify from "dompurify";
import { JSDOM } from "jsdom";

export default function Home() {
  return (
    <div
      dangerouslySetInnerHTML={{
        __html: DOMPurify(new JSDOM("<!DOCTYPE html>").window).sanitize(
          "<img src=x onerror=alert(1)//>"
        ),
      }}
    />
  );
}

There is also some required Next.js configuration for this, to resolve the webpack bundling errors:

next.config.js

/** @type {import("next").NextConfig} */
module.exports = {
  reactStrictMode: true,
+ webpack: (config) => {
+   config.externals = [...config.externals, "canvas", "jsdom"];
+   return config;
+ },
};

This works like a charm! Thank you @Daggron

thousight commented 1 year ago

For me, I'm working with Next.js 13 server component and this worked for me:

Server Component SVGIcon:

import { sanitize } from 'isomorphic-dompurify'

export default async function SVGIcon({
  src,
  width,
  height,
  className,
}: ISVGIcon) {
  const iconResponse = await fetch(`http://localhost:3000${src}`)
  const svgResult = await iconResponse.text()

  return svgResult ? (
    <div
      className={className}
      style={{ width: `${width}px`, height: `${height}px` }}
      dangerouslySetInnerHTML={{ __html: sanitize(svgResult) }}
      aria-hidden={true}
    />
  ) : null
}

next.config.js:

module.exports = {
  reactStrictMode: true,
+ webpack: (config) => {
+   config.externals = [...config.externals, "jsdom"];
+   return config;
+ },
};

Just need to install both isomorphic-dompurify and jsdom, no need to worry about canvas.

miklosz commented 1 year ago

Still valid in 1.8.0

My Next.js build warns not only about missing 'canvas', but actually 3 missing deps from js-dom already Can't resolve 'canvas' Can't resolve 'bufferutil' Can't resolve 'utf-8-validate'

Any luck with those fixes?

elisabeth0bangoura commented 1 year ago

It works now by only adding this in your next.config.js:

module.exports = { reactStrictMode: true, webpack: (config) => { config.externals = [...config.externals, "canvas", "jsdom"]; return config; }, }

and then you can copy this example in your component https://konvajs.org/docs/react/Transformer.html

works fine in : "next": "^13.4.20-canary.0"

MrUsmanDev commented 10 months ago

Whats Wrong with my code its giving me error Load failed

'use client' import React,{useEffect, useState} from 'react' import { Document, Page, pdfjs } from "react-pdf"; pdfjs.GlobalWorkerOptions.workerSrc = //unpkg.com/pdfjs-dist@${pdfjs.version}/build/pdf.worker.min.js;

const PdfViewer = ({SelectedFileData}) => { const [numPages, setNumPages] = useState(); const [pageNumber, setPageNumber] = useState(1); function onDocumentLoadSuccess({ numPages }: { numPages: number }): void { setNumPages(numPages); } return (

) }

export default PdfViewer

kkomelin commented 10 months ago

@MrUsmanDev why do you think it's related to this particular issue?

brandon-v-seeters commented 7 months ago

After trying out a lot of the suggested fixes (but for Nuxt instead of Next) we eventually switched to the following npm package: sanitize-html

After installing this package and following the instructions on the link, the builds succeeded again! :)

kkomelin commented 7 months ago

Happy to hear @brandon-v-seeters. Thanks for sharing