Exception is not a constructor since Vite 5.1.0

[jderusse]

Describe the bug

After upgrading from vite 5.0.12 to 5.1.0, I get a TypeError at runtime (in my browser's console)

Uncaught TypeError: paper.Point is not a constructor
    at index.ts:35:17
(anonymous) @ index.ts:35

paper is the library https://www.npmjs.com/package/paper installed with

Reproduction

https://stackblitz.com/edit/vitejs-vite-szaciy?file=main.js

Steps to reproduce

install paper

npm i paper

use it

// index.ts
import * as paper from 'paper/dist/paper-core'


const color = new paper.Color('#2e95c8')

See browser's logs

System Info

~/projects/vitejs-vite-szaciy
❯ npx envinfo --system --npmPackages '{vite,@vitejs/*,rollup}' --binaries --browsers
Need to install the following packages:
envinfo@7.11.1
Ok to proceed? (y) 

  System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.18.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.2.3 - /usr/local/bin/npm
    pnpm: 8.14.0 - /usr/local/bin/pnpm
  npmPackages:
    vite: ^5.1.0 => 5.1.0 

### Used Package Manager

npm

### Logs

_No response_

### Validations

- [X] Follow our [Code of Conduct](https://github.com/vitejs/vite/blob/main/CODE_OF_CONDUCT.md)
- [X] Read the [Contributing Guidelines](https://github.com/vitejs/vite/blob/main/CONTRIBUTING.md).
- [X] Read the [docs](https://vitejs.dev/guide).
- [X] Check that there isn't [already an issue](https://github.com/vitejs/vite/issues) that reports the same bug to avoid creating a duplicate.
- [X] 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](https://github.com/vuejs/core) instead.
- [X] Check that this is a concrete bug. For Q&A open a [GitHub Discussion](https://github.com/vitejs/vite/discussions) or join our [Discord Chat Server](https://chat.vitejs.dev/).
- [X] The provided reproduction is a [minimal reproducible example](https://stackoverflow.com/help/minimal-reproducible-example) of the bug.

[stackblitz]

Start a new pull request in StackBlitz Codeflow.

• vitejs-vite-szaciy (Open reproduction)

[chaejunlee]

This looks like a duplicate of it is related to #14048.

Vite community (including me) has recognized this and is working on it! If you have any suggestions, feel free to do so!

[hi-ogawa]

I think this issue is about cjs/esm interop. Probably this is a related PR:

• fix: handle namespace import and dynamic import interop consistently #15619

Vite now uses object spread in some cases (something like const paper = { ...cjsLib, default: cjsLib }), but this object spread doesn't include "inherited" properties (in this case cjsLib.__proto__.Color), so paper.Color ends up undefined. Previous simple approach was just const paper = cjsLib, so it was somewhat magically handling such inheritance.

I updated a reproduction to illustrate this with a bunch of logging https://stackblitz.com/edit/vitejs-vite-qfx5gb?file=main.js

It looks like both dev and build were working previously, so I think rollup handles this case somehow. I guess technically this was a breaking change, but I feel aligning this behavior goes against ESM concept since paper defined by import * as paper from ... should be a plain module object without inheritance. Instead, hopefully user code can adjust it by using default import for cjs:

import paper from "paper/dist/paper-core"

[XiSenao]

Thanks to @hi-ogawa for pointing out. The objects imported by the namespace import may not need to maintain the prototype chain. I think this may be an abnormal behavior of rollup, which does not apply to vite. I will temporarily close the fix until confirmation from the rollup side.

ref: rollup/rollup#5403

[jderusse]

I feel aligning this behavior goes against ESM concept since paper defined by import * as paper from ... should be a plain module object without inheritance. Instead, hopefully user code can adjust it by using default import for cjs:

import paper from "paper/dist/paper-core"

indeed, this fix the issue.

[9inpachi]

I am also experiencing the same error with one of my project's dependencies. Recently, the import for one of the transitive dependencies have changed in the project and now the default exported module is not working with import * as Stats from 'stats.js'.

See issue in @react-three/drei: pmndrs/drei#1865

@react-three/drei has "allowSyntheticDefaultImports": false, so it doesn't allow importing the module directly like import Stats from stats.js.

I have prepared a minimal stackblitz example. The import syntax in the example seems to be correct as TypeScript is able to recognize the constructor but running or building through vite fails.

See example on stackblitz: https://stackblitz.com/edit/vitejs-vite-zbir1r?file=src%2Fmain.ts

[XiSenao]

The following is an interpretation of the namespace import object form:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import#module_namespace_object

I think that the error message "is not a constructor" when instantiating a regular object is as expected.

I tried the default create-react-app template that uses webpack and the import seems to work there.

At the same time, I also tested webpack and found that the error messages it provides are consistent with vite.
https://stackblitz.com/edit/webpack-webpack-js-org-gnwump?file=src%2Findex.js

Therefore, I think vite is running as expected, so it's not a problem with vite. @9inpachi

[9inpachi]

@XiSenao

The version of webpack used in the example seems quite old.

I was able to make the import work in the latest version of webpack: https://stackblitz.com/edit/webpack-webpack-js-org-akoet6

[hi-ogawa]

@XiSenao Thanks for investigating. You closed a PR #16009 but it might be still better to have a fix since this was technically a breaking change and it seems like a new source of dev/build inconsistency.

Personally I get that namespace import with cjs is very odd in some cases, but if the fix is simple and align with rollup, then I think Vite team will consider it. If Vite team wants to make esm/cjs interop stricter, then that probably needs to postponed to major release.

@bluwy Do you have any thoughts on this issue and XiSenao's PR?

[bluwy]

Thanks for discussing this and laying out the issue. I certainly should've looked in the regression earlier 😅 Reading both sides, I think I agree with @XiSenao that this isn't a bug given the behaviour aligns with node and what's documented in MDN. Personally, I'd prioritize build to match dev compared to the other way round, as dev is where one spends most of their time on.

However, I may be persuaded to make a fix instead if other maintainers feel that this is a big breaking change. I'll ping them to hear their thoughts on this. I'd like to start merging the minor PRs today, so if this issue isn't concluded today, the fix may only land in v5.2.

EDIT: Also, would fixing this cause an inconsistency in node SSR in prod? (if the dep is not bundled)

[9inpachi]

Thanks for discussing this and laying out the issue. I certainly should've looked in the regression earlier 😅 Reading both sides, I think I agree with @XiSenao that this isn't a bug given the behaviour aligns with node and what's documented in MDN. Personally, I'd prioritize build to match dev compared to the other way round, as dev is where one spends most of their time on.

@bluwy I am not sure how types for this are correct in TypeScript. The default export becomes part of the namespace import and there is no error there. And on the other hand, ImportNamespace.default gives a type error.

According to TypeScript, using export = syntax to export default modules is preferred and should work everywhere and seems to work with namespace import with the default tsconfig, so I am confused why Vite fails. Is TypeScript in the wrong here?

See reference TypeScript docs: https://www.typescriptlang.org/docs/handbook/declaration-files/templates/module-d-ts.html#:~:text=Note%20that%20using,using%20export%3D%3A