library mode: Vite doesn't appear to transform cjs deps correctly

[frehner]

Describe the bug

I'm using Vite to bundle my library. When I bundle my library to es format, a transitive dep (that is only in cjs format) doesn't appear to be transformed correctly. Here's the setup:

my_library depends on @xstate/react/fsm, which depends on use-sync-external-store (cjs only).

It seems that use-sync-external-store isn't being correctly parsed/transformed by Vite, and is causing crashes.

Reproduction

https://stackblitz.com/edit/vitejs-vite-czfvzb?file=lib.js,package.json,vite.config.js,dist%2F_virtual%2Fwith-selector.js&terminal=dev

Steps to reproduce

Using StackBlitz above:

• run npm run build
• See contents of build/lib.js
• Follow import to "build/node_modules/@xstate/react/es/fsm.js"
  • if you can't see node_modules in StackBlitz, then the important lines from that are:

import { w as withSelector } from "../../../../_virtual/with-selector.js";
...
var storeSnapshot = withSelector.exports.useSyncExternalStoreWithSelector(subscribe, getSnapshot, getSnapshot, identity, isEqual);

• see contents of dist/_virtual/with-selector.js and note that exports are empty, so the line above fails. Here's what that file looks like for reference:

var withSelector = { exports: {} };
export {
  withSelector as w
};

---

I think this appears to be an issue with Vite, but I wouldn't mind being corrected if I just don't have something setup correctly or if my assumptions are wrong. Thanks for your time.

System Info

System:
    OS: macOS 13.0
    CPU: (8) arm64 Apple M1 Pro
    Memory: 167.80 MB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.17.0 - /opt/dev/sh/nvm/versions/node/v16.17.0/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 8.15.0 - /opt/dev/sh/nvm/versions/node/v16.17.0/bin/npm
  Browsers:
    Chrome: 107.0.5304.110
    Firefox Developer Edition: 107.0
    Safari: 16.1
    Safari Technology Preview: 16.4

Used Package Manager

yarn

Logs

No response

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to vuejs/core instead.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[sapphi-red]

I wasn't able to reproduce with the following steps.

1. npm i
2. npm run build
3. open ./dist/node_modules/@xstate/react/es/fsm.js
4. open ./dist/node_modules/use-sync-external-store/shim/with-selector.js

// ./dist/node_modules/@xstate/react/es/fsm.js
import "../../../react/index.js";
import "../../../use-sync-external-store/shim/with-selector.js";
globalThis && globalThis.__read || function(o, n) {
  var m = typeof Symbol === "function" && o[Symbol.iterator];
  if (!m)
    return o;
  var i = m.call(o), r, ar = [], e;
  try {
    while ((n === void 0 || n-- > 0) && !(r = i.next()).done)
      ar.push(r.value);
  } catch (error) {
    e = { error };
  } finally {
    try {
      if (r && !r.done && (m = i["return"]))
        m.call(i);
    } finally {
      if (e)
        throw e.error;
    }
  }
  return ar;
};
//# sourceMappingURL=fsm.js.map

// ./dist/node_modules/use-sync-external-store/shim/with-selector.js
import { w as withSelector } from "../../../_virtual/with-selector.js";
import { __require as requireWithSelector_production_min } from "../cjs/use-sync-external-store-shim/with-selector.production.min.js";
import { __require as requireWithSelector_development } from "../cjs/use-sync-external-store-shim/with-selector.development.js";
(function(module) {
  if (process.env.NODE_ENV === "production") {
    module.exports = requireWithSelector_production_min();
  } else {
    module.exports = requireWithSelector_development();
  }
})(withSelector);
//# sourceMappingURL=with-selector.js.map

It doesn't have the content you described. _virtual/* files has an empty exports but I don't think it's wrong as they have the actual content in a different file. (e.g. ./dist/node_modules/use-sync-external-store/shim/with-selector.js)

[frehner]

You're absolutely right, sorry that I missed that.

The output from StackBlitz looks different than the output for my library, and I have no idea why at the moment. I'm trying to reproduce it; I'll just close this issue and open a new one if I need to. Sorry, and thank you!

[frehner]

Ok actually I was able to reproduce it outside of StackBlitz, so I'll just reopen this issue if that's ok. (I can open a new one if you prefer)

How to reproduce:

1. clone this repo https://github.com/frehner/vite-lib-mode
2. npm i
3. npm run build

Note that I committed the built files to the repo, so you don't technically need to run anything to see the output. For example, you will now see that the built ./dist/dev/node_modules/@xstate/react/es/fsm.mjs file has the problematic import

https://github.com/frehner/vite-lib-mode/blob/97b4eb3ef2b9796a00b0bcb38b3f5c8912cc5337/dist/dev/node_modules/%40xstate/react/es/fsm.js#L8

that causes things to break when it reaches this line

https://github.com/frehner/vite-lib-mode/blob/97b4eb3ef2b9796a00b0bcb38b3f5c8912cc5337/dist/dev/node_modules/%40xstate/react/es/fsm.js#L89

below.

I have yet to figure out why StackBlitz wouldn't output things this way, but this example repo does.

[sapphi-red]

@frehner Your repository seems to be private.

[frehner]

Ack, I'm so sorry. Should be fixed.

[sapphi-red]

I think the output is correct.

../../../../_virtual/with-selector.js has an empty object (exports.withSelector = { exports: {} }).
But the content will be included before because of require("../../../use-sync-external-store/shim/with-selector.js");.
https://github.com/frehner/vite-lib-mode/blob/97b4eb3ef2b9796a00b0bcb38b3f5c8912cc5337/dist/dev/node_modules/%40xstate/react/es/fsm.js#L6-L8
node_modules/use-sync-external-store/shim/with-selector.js will do withSelector.exports = require(/* */).
https://github.com/frehner/vite-lib-mode/blob/97b4eb3ef2b9796a00b0bcb38b3f5c8912cc5337/dist/dev/node_modules/use-sync-external-store/shim/with-selector.js#L2-L11

[frehner]

Is that true of the esm files too? Because I've found that my library works fine for cjs, but esm mode breaks. (You're linking to the cjs versions)

I'll get a reproduction with NextJS as an example up soon

[sapphi-red]

Ah, I mistakenly read the cjs version but esm has a similar code and will work.

Assuming that the library you are talking about is hydrogen-ui, I guess this sideEffects field is wrong and removing that or specifying correctly will work.
https://github.com/Shopify/hydrogen-ui/blob/f1cb723b81cf013da316eb7fa115822779ad0cbc/packages/react/package.json#L57

[frehner]

Yup, talking about hydrogen-ui! Here's a PR / repo of things failing in NextJS with these settings Shopify/hydrogen-react#64

I guess this sideEffects field is wrong and removing that or specifying correctly will work.

Is... there something that should be included in there that I'm missing? Are there built files that should be included?

[edit] but you're right, by removing it NextJS works! So there must be some built files that I need to add to it?

[frehner]

Since it's not feasible to completely remove sideEffects from package.json (otherwise, my library couldn't be tree-shaked, right?), I guess for now I'll just have to keep an eye on built files and include them if they pop up?

I think by changing it to:

{
"sideEffects": [
    "dist/dev/node_modules/use-sync-external-store/shim/with-selector.mjs",
    "dist/prod/node_modules/use-sync-external-store/shim/with-selector.mjs",
    "dist/dev/node_modules/use-sync-external-store/shim/with-selector.js",
    "dist/prod/node_modules/use-sync-external-store/shim/with-selector.js"
  ],
}

that will cover it for now? Not a very scalable solution as we'll have to track things down manually if they crop up, but works for now I think.

[sapphi-red]

Is... there something that should be included in there that I'm missing? Are there built files that should be included?

At least, dist/dev/node_modules/use-sync-external-store/shim/with-selector.mjs has a side effect.
If this file is imported, withSelector exported from dist/dev/_virtual/with-selector.mjs will be modified.
sideEffects field means that that file can be removed if no export was used. But in this case, this file does have a side effect of modifying the value of dist/dev/_virtual/with-selector.mjs.

I guess for now I'll just have to keep an eye on built files and include them if they pop up?

I'm not sure how much Webpack can tree shake but I guess yes.

That will cover. If you don't want to check whether the bundled package has a side effect (as they aren't in control), I think changing to this is reasonable.

{
  "sideEffects": [
    "dist/*/node_modules/**/*"
  ],
}

[frehner]

Thank you so much for your help.

It would be a great update if Vite could change how it emits those files so that they don't depend on side effects, but the workaround for now is doable.

We can probably close this issue, unless you want to keep it open and change it to be that enhancement request. Or we can close this and I can open a separate issue asking for that.

Which would you prefer?

[sapphi-red]

This seems to reproduce with rollup + @rollup/plugin-commonjs + @rollup/plugin-node-resolve.
https://stackblitz.com/edit/rollup-template-1kqybv?file=rollup.config.js

So I think this enhancement request will be in rollup/plugins repo instead of here.