Skip to content

Commit 228539f

Browse files
owlstronautowlstronaut
authored
feat: adds fixName step for publishing (#145)
1 parent b6465f4 commit 228539f

5 files changed

Lines changed: 225 additions & 10 deletions

File tree

‎lib/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ class PackageJson {
4141
'binRefs',
4242
'bundleDependencies',
4343
'bundleDependenciesFalse',
44+
'fixName',
4445
'fixNameField',
4546
'fixVersionField',
4647
'fixRepositoryField',

‎lib/normalize.js‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ const clean = require('semver/functions/clean')
33
const fs = require('node:fs/promises')
44
const path = require('node:path')
55
const { log } = require('proc-log')
6+
const moduleBuiltin = require('node:module')
67

78
/**
89
* @type {import('hosted-git-info')}
@@ -144,7 +145,7 @@ const normalize = async (pkg, { strict, steps, root, changes, allowLegacyCase })
144145
const pkgId = `${data.name ?? ''}@${data.version ?? ''}`
145146

146147
// name and version are load bearing so we have to clean them up first
147-
if (steps.includes('fixNameField') || steps.includes('normalizeData')) {
148+
if (steps.includes('fixName') || steps.includes('fixNameField') || steps.includes('normalizeData')) {
148149
if (!data.name && !strict) {
149150
changes?.push('Missing "name" field was set to an empty string')
150151
data.name = ''
@@ -170,6 +171,13 @@ const normalize = async (pkg, { strict, steps, root, changes, allowLegacyCase })
170171
}
171172
}
172173

174+
if (steps.includes('fixName')) {
175+
// Check for conflicts with builtin modules
176+
if (moduleBuiltin.builtinModules.includes(data.name)) {
177+
log.warn('package-json', pkgId, `Package name "${data.name}" conflicts with a Node.js built-in module name`)
178+
}
179+
}
180+
173181
if (steps.includes('fixVersionField') || steps.includes('normalizeData')) {
174182
// allow "loose" semver 1.0 versions in non-strict mode
175183
// enforce strict semver 2.0 compliance in strict mode

‎tap-snapshots/test/fix.js.test.cjs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,21 @@ Array [
6969
]
7070
`
7171

72-
exports[`test/fix.js TAP with changes fixNameField scoped whitespace > must match snapshot 1`] = `
72+
exports[`test/fix.js TAP with changes fixName step allows uppercase in package name > must match snapshot 1`] = `
73+
Array []
74+
`
75+
76+
exports[`test/fix.js TAP with changes fixName step warning for builtin module name > must match snapshot 1`] = `
77+
Array []
78+
`
79+
80+
exports[`test/fix.js TAP with changes fixNameField scoped package name with whitespace > must match snapshot 1`] = `
7381
Array [
7482
"Whitespace was trimmed from \\"name\\"",
7583
]
7684
`
7785

78-
exports[`test/fix.js TAP with changes fixNameField unscoped whitespace > must match snapshot 1`] = `
86+
exports[`test/fix.js TAP with changes fixNameField unscoped package name with whitespace > must match snapshot 1`] = `
7987
Array [
8088
"Whitespace was trimmed from \\"name\\"",
8189
]

‎test/fix-name.js‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
const t = require('tap')
2+
const PackageJson = require('../')
3+
const normalize = require('../lib/normalize.js')
4+
5+
const pkg = (data = {}) => {
6+
return JSON.stringify({
7+
name: '@npmcli/test-package',
8+
version: '1.0.0',
9+
...data,
10+
})
11+
}
12+
13+
// Helper to test the fixName step
14+
const testFixNameStep = async (t, testdir, expectation) => {
15+
const p = t.testdir(testdir)
16+
17+
// Test using normalize directly with fixName step
18+
const instance = await PackageJson.load(p)
19+
await normalize(instance, { steps: ['fixName'], strict: true, allowLegacyCase: true })
20+
21+
// Apply expectation function if provided
22+
if (expectation) {
23+
expectation(t, instance.content)
24+
}
25+
26+
return instance
27+
}
28+
29+
t.test('fixName step', async t => {
30+
t.test('valid package name passes validation', async t => {
31+
const testdir = {
32+
'package.json': pkg({ name: '@npmcli/test-package' }),
33+
}
34+
35+
await testFixNameStep(t, testdir, (t, content) => {
36+
t.strictSame(content.name, '@npmcli/test-package', 'name should remain unchanged')
37+
})
38+
})
39+
40+
t.test('missing name field throws error', async t => {
41+
const testdir = {
42+
'package.json': pkg({ name: undefined }),
43+
}
44+
45+
await t.rejects(
46+
async () => {
47+
const instance = await PackageJson.load(t.testdir(testdir))
48+
await normalize(instance, { steps: ['fixName'], strict: true, allowLegacyCase: true })
49+
},
50+
{ message: /name field must be a string/ },
51+
'should reject with appropriate error message'
52+
)
53+
})
54+
55+
t.test('non-string name field throws error', async t => {
56+
const testdir = {
57+
'package.json': pkg({ name: ['@npmcli/invalid-test-package'] }),
58+
}
59+
60+
await t.rejects(
61+
async () => {
62+
const instance = await PackageJson.load(t.testdir(testdir))
63+
await normalize(instance, { steps: ['fixName'], strict: true, allowLegacyCase: true })
64+
},
65+
{ message: /name field must be a string/ },
66+
'should reject with appropriate error message'
67+
)
68+
})
69+
70+
t.test('invalid package name formats throw error', async t => {
71+
t.test('leading dot', async t => {
72+
const testdir = {
73+
'package.json': pkg({ name: '.npmcli-test-package' }),
74+
}
75+
76+
await t.rejects(
77+
async () => {
78+
const instance = await PackageJson.load(t.testdir(testdir))
79+
await normalize(instance, { steps: ['fixName'], strict: true, allowLegacyCase: true })
80+
},
81+
{ message: /Invalid name/ },
82+
'should reject names starting with a dot'
83+
)
84+
})
85+
86+
t.test('invalid scoped package name format', async t => {
87+
const testdir = {
88+
'package.json': pkg({ name: '@npmcli/test/package/extra' }),
89+
}
90+
91+
await t.rejects(
92+
async () => {
93+
const instance = await PackageJson.load(t.testdir(testdir))
94+
await normalize(instance, { steps: ['fixName'], strict: true, allowLegacyCase: true })
95+
},
96+
{ message: /Invalid name/ },
97+
'should reject invalid scoped package names'
98+
)
99+
})
100+
101+
t.test('node_modules reserved name', async t => {
102+
const testdir = {
103+
'package.json': pkg({ name: 'node_modules' }),
104+
}
105+
106+
await t.rejects(
107+
async () => {
108+
const instance = await PackageJson.load(t.testdir(testdir))
109+
await normalize(instance, { steps: ['fixName'], strict: true, allowLegacyCase: true })
110+
},
111+
{ message: /Invalid name/ },
112+
'should reject reserved name node_modules'
113+
)
114+
})
115+
116+
t.test('favicon.ico reserved name', async t => {
117+
const testdir = {
118+
'package.json': pkg({ name: 'favicon.ico' }),
119+
}
120+
121+
await t.rejects(
122+
async () => {
123+
const instance = await PackageJson.load(t.testdir(testdir))
124+
await normalize(instance, { steps: ['fixName'], strict: true, allowLegacyCase: true })
125+
},
126+
{ message: /Invalid name/ },
127+
'should reject reserved name favicon.ico'
128+
)
129+
})
130+
})
131+
132+
t.test('builtin module name conflict', async t => {
133+
const builtinModules = require('node:module').builtinModules
134+
const builtinName = builtinModules[0] // Use the first builtin module name
135+
136+
const testdir = {
137+
'package.json': pkg({ name: builtinName }),
138+
}
139+
140+
// For builtin modules, we don't throw but we log a warning
141+
const instance = await testFixNameStep(t, testdir, (t, content) => {
142+
t.strictSame(content.name, builtinName, 'should allow builtin module names')
143+
})
144+
145+
// We can't directly test the warning since it's logged, but we can confirm the name remains unchanged and the operation completes without error
146+
t.ok(instance, 'operation should complete without error')
147+
})
148+
149+
t.test('package with uppercase name', async t => {
150+
const testdir = {
151+
'package.json': pkg({ name: '@NPMCLI/Test-Package' }),
152+
}
153+
154+
// Case is allowed
155+
await testFixNameStep(t, testdir, (t, content) => {
156+
t.strictSame(content.name, '@NPMCLI/Test-Package', 'should allow uppercase in package name for publishing')
157+
})
158+
})
159+
})

‎test/fix.js‎

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -105,19 +105,58 @@ for (const [name, testFix] of Object.entries(testMethods)) {
105105
{ message: /Invalid name/ }
106106
)
107107
})
108-
t.test('scoped whitespace', async t => {
108+
t.test('scoped package name with whitespace', async t => {
109109
const testdir = {
110110
'package.json': pkg({ name: '@npmcli/test-package ' }),
111111
}
112-
const { content } = await testFix(t, testdir)
113-
t.strictSame(content.name, '@npmcli/test-package')
112+
113+
// When using fixNameField, whitespace should be trimmed
114+
const fixed = await testFix(t, testdir, { steps: ['fixNameField'] })
115+
t.strictSame(fixed.content.name, '@npmcli/test-package', 'whitespace should be trimmed')
114116
})
115-
t.test('unscoped whitespace', async t => {
117+
t.test('unscoped package name with whitespace', async t => {
116118
const testdir = {
117-
'package.json': pkg({ name: '@npmcli/test-package ' }),
119+
'package.json': pkg({ name: 'npmcli-test-package ' }),
118120
}
119-
const { content } = await testFix(t, testdir)
120-
t.strictSame(content.name, '@npmcli/test-package')
121+
122+
// When using fixNameField, whitespace should be trimmed
123+
const fixed = await testFix(t, testdir, { steps: ['fixNameField'] })
124+
t.strictSame(fixed.content.name, 'npmcli-test-package', 'whitespace should be trimmed')
125+
})
126+
})
127+
t.test('fixName step', async t => {
128+
t.test('warning for builtin module name', async t => {
129+
const builtinModules = require('node:module').builtinModules
130+
const builtinName = builtinModules[0] // Use the first builtin module name
131+
132+
const testdir = {
133+
'package.json': pkg({ name: builtinName }),
134+
}
135+
136+
// Should not throw error since this is just a warning
137+
const { content } = await testFix(t, testdir, { steps: ['fixName'], strict: true, allowLegacyCase: true })
138+
t.strictSame(content.name, builtinName, 'should allow but warn about builtin module name')
139+
})
140+
141+
t.test('should reject invalid package name', async t => {
142+
const testdir = {
143+
'package.json': pkg({ name: '.invalid-name' }),
144+
}
145+
146+
await t.rejects(
147+
testFix(t, testdir, { steps: ['fixName'], strict: true, allowLegacyCase: true }),
148+
{ message: /Invalid name/ }
149+
)
150+
})
151+
152+
t.test('allows uppercase in package name', async t => {
153+
const testdir = {
154+
'package.json': pkg({ name: '@NPMCLI/Test-Package' }),
155+
}
156+
157+
// With fixName, uppercase is allowed
158+
const { content } = await testFix(t, testdir, { steps: ['fixName'], strict: true, allowLegacyCase: true })
159+
t.strictSame(content.name, '@NPMCLI/Test-Package', 'should allow uppercase in package name')
121160
})
122161
})
123162
t.test('fixVersionField', async t => {

0 commit comments

Comments
 (0)