Skip to content

Commit 69a4856

Browse files
sxzzsxzz
committed
fix: don't strip node-protocol-only module
closes #482, closes #483
1 parent 039e276 commit 69a4856

10 files changed

+91
-91
lines changed

‎src/features/node-protocol.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,23 @@
11
import { builtinModules } from 'node:module'
22
import type { Plugin } from 'rolldown'
33

4+
const modulesWithoutProtocol = builtinModules.filter(
5+
(mod) => !mod.startsWith('node:'),
6+
)
7+
48
/**
59
* The `node:` protocol was added in Node.js v14.18.0.
610
* @see https://nodejs.org/api/esm.html#node-imports
711
*/
812
export function NodeProtocolPlugin(nodeProtocolOption: 'strip' | true): Plugin {
913
if (nodeProtocolOption === 'strip') {
14+
const regex = new RegExp(`^node:(${modulesWithoutProtocol.join('|')})$`)
15+
1016
return {
1117
name: 'tsdown:node-protocol:strip',
1218
resolveId: {
1319
order: 'pre',
14-
filter: { id: /^node:/ },
20+
filter: { id: regex },
1521
handler(id) {
1622
return {
1723
id: id.slice(5), // strip the `node:` prefix
@@ -25,7 +31,9 @@ export function NodeProtocolPlugin(nodeProtocolOption: 'strip' | true): Plugin {
2531

2632
// create regex from builtin modules
2733
// filter without `node:` prefix
28-
const builtinModulesRegex = new RegExp(`^(${builtinModules.join('|')})$`)
34+
const builtinModulesRegex = new RegExp(
35+
`^(${modulesWithoutProtocol.join('|')})$`,
36+
)
2937

3038
return {
3139
name: 'tsdown:node-protocol:add',

‎src/options/index.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,12 @@ async function resolveConfig(
215215
logger.warn('`bundle` option is deprecated. Use `unbundle` instead.')
216216
}
217217

218+
if (removeNodeProtocol && nodeProtocol) {
219+
throw new TypeError(
220+
'`removeNodeProtocol` is deprecated. Please only use `nodeProtocol` instead.',
221+
)
222+
}
223+
218224
// Resolve nodeProtocol option with backward compatibility for removeNodeProtocol
219225
nodeProtocol =
220226
nodeProtocol ??

‎src/options/types.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -213,9 +213,9 @@ export interface Options {
213213
removeNodeProtocol?: boolean
214214

215215
/**
216-
* - If true, add `node:` prefix to built-in modules.
217-
* - If 'strip', strips the `node:` protocol prefix from import source.
218-
* - If false, does not modify the import source.
216+
* - If `true`, add `node:` prefix to built-in modules.
217+
* - If `'strip'`, strips the `node:` protocol prefix from import source.
218+
* - If `false`, does not modify the import source.
219219
*
220220
* @default false
221221
*

‎tests/__snapshots__/node-protocol/nodeProtocol-option-takes-precedence-over-removeNodeProtocol.snap.md‎

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎tests/__snapshots__/node-protocol/should-handle-modules-that-require-node-prefix.snap.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
## index.js
2+
3+
```js
4+
import test from "node:test";
5+
import sqlite from "node:sqlite";
6+
7+
export { sqlite, test };
8+
```

‎tests/__snapshots__/node-protocol/should-not-double-prefix-modules-that-already-have-node-prefix.snap.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
## index.js
2+
3+
```js
4+
import fs from "node:fs";
5+
import { join } from "node:path";
6+
import * as crypto from "node:crypto";
7+
import * as nodeSqlite from "node:sqlite";
8+
import * as sqlite from "sqlite";
9+
10+
export { crypto, fs, join, nodeSqlite, sqlite };
11+
```

‎tests/__snapshots__/remove-node-protocol/basic.snap.md‎

Lines changed: 0 additions & 12 deletions
This file was deleted.

‎tests/__snapshots__/remove-node-protocol/w-require-polyfill.snap.md‎

Lines changed: 0 additions & 15 deletions
This file was deleted.

‎tests/node-protocol.test.ts‎

Lines changed: 53 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { describe, expect, test } from 'vitest'
2+
import { resolveOptions } from '../src/options'
23
import { testBuild } from './utils'
34

45
describe('node protocol', () => {
@@ -103,22 +104,15 @@ describe('node protocol', () => {
103104
expect(snapshot).not.contains('node:')
104105
})
105106

106-
test('nodeProtocol option takes precedence over removeNodeProtocol', async (context) => {
107-
const files = {
108-
'index.ts': `
109-
import fs from 'fs'
110-
export { fs }
111-
`,
112-
}
113-
const { snapshot } = await testBuild({
114-
context,
115-
files,
116-
options: {
107+
test('nodeProtocol option takes precedence over removeNodeProtocol', async () => {
108+
await expect(() =>
109+
resolveOptions({
117110
nodeProtocol: true,
118111
removeNodeProtocol: true,
119-
},
120-
})
121-
expect(snapshot).toMatch(/from ['"]node:fs['"]/)
112+
}),
113+
).rejects.toThrowError(
114+
`\`removeNodeProtocol\` is deprecated. Please only use \`nodeProtocol\` instead.`,
115+
)
122116
})
123117

124118
test('mixed imports with nodeProtocol: true', async (context) => {
@@ -198,4 +192,49 @@ describe('node protocol', () => {
198192
expect(snapshot).toMatch(/from ['"]node:fs\/promises['"]/)
199193
expect(snapshot).toMatch(/from ['"]node:url['"]/)
200194
})
195+
196+
test('should not double-prefix modules that already have node: prefix', async (context) => {
197+
const files = {
198+
'index.ts': `
199+
import fs from 'fs'
200+
import { join } from 'node:path'
201+
import * as crypto from 'node:crypto'
202+
import * as nodeSqlite from 'node:sqlite'
203+
import * as sqlite from 'sqlite'
204+
export { fs, join, crypto, nodeSqlite, sqlite }
205+
`,
206+
}
207+
const { snapshot } = await testBuild({
208+
context,
209+
files,
210+
options: {
211+
nodeProtocol: true,
212+
},
213+
})
214+
215+
expect(snapshot).toMatch(/nodeSqlite from ['"]node:sqlite['"]/)
216+
expect(snapshot).toMatch(/sqlite from ['"]sqlite['"]/)
217+
expect(snapshot).not.includes('node:node:')
218+
})
219+
220+
test('should handle modules that require node: prefix', async (context) => {
221+
// Simulate modules that only exist with node: prefix
222+
const files = {
223+
'index.ts': `
224+
import test from 'node:test'
225+
import sqlite from 'node:sqlite'
226+
export { test, sqlite }
227+
`,
228+
}
229+
const { snapshot } = await testBuild({
230+
context,
231+
files,
232+
options: {
233+
nodeProtocol: 'strip',
234+
},
235+
})
236+
// For node:-only modules, the prefix should be preserved even in strip mode
237+
expect(snapshot).toMatch(/from ['"]node:test['"]/)
238+
expect(snapshot).toMatch(/from ['"]node:sqlite['"]/)
239+
})
201240
})

‎tests/remove-node-protocol.test.ts‎

Lines changed: 0 additions & 38 deletions
This file was deleted.

0 commit comments

Comments
 (0)