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

Issue after upgrade using jsdom with optional dependencies #10255

Closed
7 tasks done
robpc opened this issue Sep 27, 2022 · 6 comments · Fixed by #10593
Closed
7 tasks done

Issue after upgrade using jsdom with optional dependencies #10255

robpc opened this issue Sep 27, 2022 · 6 comments · Fixed by #10593
Labels
feat: ssr pending triage regression The issue only appears after a new release

Comments

@robpc
Copy link

robpc commented Sep 27, 2022

Describe the bug

I noticed an issue after upgrading an ssr project with vite from 3.0.4 to 3.1.3. Using jsdom in a server side page results in an error about an optional dependency.

Error: ENOENT: no such file or directory, open '__vite-optional-peer-dep:canvas:jsdom'
    at Object.openSync (node:fs:585:3)
    at Object.readFileSync (node:fs:453:35)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1122:18)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (./node_modules/jsdom/lib/jsdom/utils.js:158:18)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1159:10)

This error was not present before the upgrade and the optional dependency is not used in the project. If you look at the jsdom line referenced above, you see

exports.Canvas = null;
let canvasInstalled = false;
try {
  require.resolve('canvas');
  canvasInstalled = true;
} catch (e) {
  // canvas is not installed
}
if (canvasInstalled) {
  ////// 👇 This is the line that causes the error //////
  const Canvas = require('canvas');
  if (typeof Canvas.createCanvas === 'function') {
    // In browserify, the require will succeed but return an empty object
    exports.Canvas = Canvas;
  }
}

Which if you replace the line require.resolve('canvas'); with something like

const r = require.resolve('canvas');
console.log('I RESOLVED CANVAS', JSON.stringify(r, null, 2));

You will get

I RESOLVED CANVAS "__vite-optional-peer-dep:canvas:jsdom"

Obviously that is not what jsdom is expecting and causes it to enter the block that requires the missing module.

I reproduced this issue by using the starter template npm create vite-extra@latest -- --template ssr-vanilla and installing jsdom and adding an import statement to the entry-server.js file. Here is a link to that simple project with the issue. Downgrading the version of vite to 3.0.4 removes the error.

Reproduction

https://github.com/robpc/vite-ssr-optional-dependency-issue

System Info

System:
    OS: macOS 12.6
    CPU: (8) x64 Intel(R) Core(TM) i7-1068NG7 CPU @ 2.30GHz
    Memory: 32.01 MB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.16.0 - ~/.nvm/versions/node/v16.16.0/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 8.11.0 - ~/.nvm/versions/node/v16.16.0/bin/npm
  Browsers:
    Chrome: 105.0.5195.125
    Firefox: 104.0.1
    Safari: 16.0
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.76
    @sveltejs/kit: next => 1.0.0-next.491
    svelte: ^3.44.0 => 3.50.1
    vite: ^3.1.0 => 3.1.3

Used Package Manager

npm

Logs

No response

Validations

@robpc
Copy link
Author

robpc commented Oct 5, 2022

Looking through the code it appears that this code is the issue, which was added in #9321

if (id.startsWith(optionalPeerDepId)) {
if (isProduction) {
return `export default {}`
} else {
const [, peerDep, parentDep] = id.split(':')
return `throw new Error(\`Could not resolve "${peerDep}" imported by "${parentDep}". Is it installed?\`)`
}
}

Looking at the node documentation that appears to break the expected behavior for require.resolve which explains what I saw in jsdom.

Use the internal require() machinery to look up the location of a module, but rather than loading the module, just return the resolved filename.

If the module can not be found, a MODULE_NOT_FOUND error is thrown.

In reviewing the issue it is trying to fix #6007 it seems a bit ironic that it is trying to fix a similar issue. I wonder if the solution there would be to ask any libraries that use optional deps and cause #6007 to test with require.resolve like jsdom does instead of having vite create a fake module that don't exist as #9321 does. I feel that has the potential to break things in other ways in addition to the one in this issue.

@harrytran998
Copy link

I have the same issue with this 🥹 - Do you have someway to workaround this? @robpc

@harrytran998
Copy link

After tracing the issue, I think you could use same lib with me isomorphic-dompurify --> You can replace that with sanitize-html lib.

References here: kkomelin/isomorphic-dompurify#54 (comment)

@blahbleepew
Copy link

I'm affected too

@harrytran998 sanitize-html isn't supposed to be used on the client

@robpc
Copy link
Author

robpc commented Oct 17, 2022

Thank you for sharing your solution @harrytran998, but that does not work for my case. I need something that mimics the document object and the dom.

@einarpersson
Copy link

Same here, using

"vite": "3.1.8",
"@sveltejs/kit": "1.0.0-next.516",
"jsdom": "20.0.1"

@github-actions github-actions bot locked and limited conversation to collaborators Nov 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat: ssr pending triage regression The issue only appears after a new release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants