valtio: vite in peerDependencies and peerDependenciesMeta causes issues in pnpm workspace

Summary

Valtio’s package.json lists vite in its peerDeps. This is unnecessary, and it has the unwanted side effect that pnpm - as a documented behavior - adds vite in valtio’s symlink folder name whenever vite is also present in a workspace package. The problem is that depending of this circumstance, the valtio folder name is non-deterministic, which leads to double instances, which again leads to a Symbol-related bug in valtio.

Link to reproduction

It’s an intranet app, can’t link it.

Check List

Please do not ask questions in issues.

  • I’ve already opened a discussion before opening this issue.

Here is an exact, reproducible scenario which leads to a runtime bug:

  1. Create a pnpm workspace with 2 packages: app and common
  2. Make common depend on vite
  3. But make sure that app does NOT depend on vite (app is using common’s build interface, or it’s using a different bundler, whatever)
  4. Also make both app and common depend on valtio
  5. Use valtio in app or common (it doesn’t matter) like this: const store = proxyWithComputed({}, {})
  6. Result: valtio will throw a runtime error saying that it can’t determine the target as a proxy object.

The explanation of the bug is as follows:

Because valtio has peer deps which are varyingly present in app and common (let’s say both are react packages, but, again, only common depends on vite), there will be 2 separate valtio folders in the pnpm store: valtio@ver+react@ver (for app) and valtio@ver+react@ver+vite@ver (for common). When there’s a call to proxyWithComputed, vite resolves its import path valtio/utils to one of the folders, but then proxyWithComputed imports valtio, which vite resolves to the other folder. This leads to the effect that proxyWithComputed doesn’t recognize the javascript Symbol on the proxy which valtio produced.

I couldn’t determine why Vite resolves one of the import specifiers (valtio/utils) to one of the folders and the other (valtio) to the other, so there might be possibly a Vite bug here, but I concluded that in the meantime, valtio’s listing of vite in its peer deps seems unreasonable, and it hijacks the pnpm behavior.

There’s a workaround for the problem, by adding a .pnpmfile.cjs which cancels valtio’s peerDeps entirely:

function readPackage(pkg, context) {
  if (pkg.name === 'valtio') {
    pkg.peerDependencies = {}
    pkg.peerDependenciesMeta = {}
  }

  return pkg
}

module.exports = {
  hooks: {
    readPackage
  }
}

Note that it would be enough to just delete vite from peerDeps, I just made it simpler. Also note that it’s necessary to fix both peerDependencies and peerDependenciesMeta.

About this issue

  • Original URL
  • State: closed
  • Created a year ago
  • Comments: 15 (15 by maintainers)

Most upvoted comments

Yeah, I think the PR is fine and make things better. We will see if that can cause any trouble with valtio/macro/vite users. Presumably not.

Another note: import {} from '../vanilla' is translated to import {} from 'valtio/vanilla' with rollup.