fix!: use type module (revert #1411)

[bluwy]

Reverts #1411 (and also #1439)

Ref: #1411 (comment)

It's not needed to remove type module to fix the issue stated in the PR

[netlify]

✅ Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	0f2f0ba
🔍 Latest deploy log	https://app.netlify.com/sites/vitest-dev/deploys/62a988d064e33a00090f3001
😎 Deploy Preview	https://deploy-preview-1465--vitest-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[sheremet-va]

Will it revert errors for users? If so, what should we recommend people?

[bluwy]

Yes it will revert the errors. Following #1411 (comment), if they're using the new nodenext or node16 options, they must name the test files with .mts if "type": "module" is not specified.

[sheremet-va]

Yes it will revert the errors. Following #1411 (comment), if they're using the new nodenext or node16 options, they must name the test files with .mts if "type": "module" is not specified.

In that case, should we also extend our defaults?

vitest/packages/vitest/src/defaults.ts

Lines 15 to 16 in fef00a7

	'test{,-*}.{js,cjs,mjs,ts,tsx,jsx}',
	'**/*{.,-}test.{js,cjs,mjs,ts,tsx,jsx}',

vitest/packages/vitest/src/defaults.ts

Line 33 in fef00a7

extension: ['.js', '.cjs', '.mjs', '.ts', '.tsx', '.jsx', '.vue', '.svelte'],

[bluwy]

I think that makes sense 👍

[antfu]

/cc @aleclarson

[antfu]

I would hold this for a while longer. Indeed the usage in #1411 might better use .mts for the tests. But as .mts is not wildly supported yet, plus there is not too much harm for us to use .mjs, I would more lean to make it easy for users until we really encounter some blockers for the current workaround.

[aleclarson]

Following #1411 (comment), if they're using the new nodenext or node16 options, they must name the test files with .mts if "type": "module" is not specified.

The "better solution" I mentioned in that thread goes like this:

".": {
  "require": {
    "types": "./dist/index.d.cts",
    "default": "./dist/index.cjs"
  },
  "types": "./dist/index.d.ts",
  "import": "./dist/index.js"
},

That would be how Vitest's exports should look. Then users aren't required to change their setup at all.

Both dist/index.d.cts and dist/index.cjs can just re-export from the other dist files.

[sheremet-va]

The "better solution" I mentioned in that thread goes like this:

First of all, we already saw in that PR that it doesn't work, does it? TypeScript has a bug preventing that.

Second of all, I don't see how this is a "better solution"? We will essentially double package size just to please few people with TypeScript issues that can be resolved on their end? Those files won't even be run in any situation.

[aleclarson]

First of all, we already saw in that PR that it doesn't work, does it? TypeScript has a big preventing that.

The two differences from your idea are that (1) require.types is defined and (2) it uses a .d.cts extension. Both are required for TypeScript to believe the package is not ESM only.

We will essentially double package size

No, the new .cts and .cjs files are basic re-exports of the ESM dist files.

// dist/index.d.cts
export * from './index.mjs'

// dist/index.cjs
export * from './index.mjs'

[sheremet-va]

No, the new .cts and .cjs files are basic re-exports of the ESM dist files.

But this wouldn't work for the same reason it doesn't work right now, no? You cannot use static import/export inside cjs.

This is also just a hack to please typescript. They added this extensions for a the reason we are trying to fix here, so I would rather go with it.

[aleclarson]

But this wouldn't work for the same reason it doesn't work right now, no? You cannot use static import/export inside cjs.

Those modules would never be loaded by Node.js runtime, so no errors would occur.

Correction: The dist/index.cjs module should actually be empty. The dist/index.d.cts allows import/export syntax, and I've tested it myself, so I know it works. :)

It's not a "hack", because the TypeScript team themselves said it's a valid solution.

[sheremet-va]

Those modules would never be loaded by Node.js runtime, so no errors would occur.

No, I mean by TypeScript. Why would importing ESM from commonjs would work inside a node_module, but not in your own project?

Current problem:

• importing Vitest from commonjs gives error from TypeScript typechecker, because it will compile import to require and requiring Vitest is not allowed
• but importing inside Vitest's cts magically works? I don't buy it tbh

[sheremet-va]

Ok, I've looked through the threads, and now I see that @aleclarson's idea is valid. We can include it in this PR actually, and safely merge it. @antfu, what do you think?

[antfu]

I can't really follow the topic, but sure, if it helps and doesn't break anything else :)

[jroru]

Is there anything blocking this from being merged?

[sheremet-va]

Is there anything blocking this from being merged?

This needs updating, we changed quite a few APIs since then, and if merged, Vitest won't work.

[jroru]

For a package with:

• package.json type set to module
• tsconfig.json compilerOptions.moduleResolution set to Node16
• vitest@0.22.1
• vite@3.0.9
• typescript@4.7.4

Running tsc outputs:

../../node_modules/vitest/dist/config.d.ts:2:27 - error TS1471: Module 'vite' cannot be imported using this construct. The specifier only resolves to an ES module, which cannot be imported synchronously. Use dynamic import instead.

2 export { ConfigEnv } from 'vite'

Is there a known workaround?

[bluwy]

I've updated the PR. Tried to update some of the newer places, and the tests seems to be passing, except ubuntu node14 (not sure if it's a fluke).

[StevenGBrown]

For a package with:

• package.json type set to module
• tsconfig.json compilerOptions.moduleResolution set to Node16
• vitest@0.22.1
• vite@3.0.9
• typescript@4.7.4

Running tsc outputs:

../../node_modules/vitest/dist/config.d.ts:2:27 - error TS1471: Module 'vite' cannot be imported using this construct. The specifier only resolves to an ES module, which cannot be imported synchronously. Use dynamic import instead.

2 export { ConfigEnv } from 'vite'

Is there a known workaround?

@jroru My workaround until this PR is merged has been to use patch-package to add "type": "module" to the vitest package.json.