Skip to content

Commit fa735ad

Browse files
antongolubantongolub
committed
fix: checks node_modules ref on linking
continues #1349
1 parent 9ef6d3c commit fa735ad

File tree

4 files changed

+127
-59
lines changed

4 files changed

+127
-59
lines changed

‎.size-limit.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@
6666
"README.md",
6767
"LICENSE"
6868
],
69-
"limit": "874.6 kB",
69+
"limit": "874.70 kB",
7070
"brotli": false,
7171
"gzip": false
7272
}

‎build/cli.cjs‎

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -218,11 +218,9 @@ function main() {
218218
});
219219
}
220220
var rmrf = (p) => {
221+
var _a2;
221222
if (!p) return;
222-
try {
223-
import_index.fs.lstatSync(p).isSymbolicLink() ? import_index.fs.unlinkSync(p) : import_index.fs.rmSync(p, { force: true, recursive: true });
224-
} catch (e) {
225-
}
223+
((_a2 = lstat(p)) == null ? void 0 : _a2.isSymbolicLink()) ? import_index.fs.unlinkSync(p) : import_index.fs.rmSync(p, { force: true, recursive: true });
226224
};
227225
function runScript(script, scriptPath, tempPath) {
228226
return __async(this, null, function* () {
@@ -238,16 +236,7 @@ function runScript(script, scriptPath, tempPath) {
238236
}
239237
const cwd = import_index.path.dirname(scriptPath);
240238
if (typeof argv.preferLocal === "string") {
241-
linkNodeModules(cwd, argv.preferLocal);
242-
try {
243-
const aliasPath = import_index.path.resolve(cwd, "node_modules");
244-
if (import_index.fs.existsSync(aliasPath) && import_index.fs.lstatSync(aliasPath).isSymbolicLink()) {
245-
nmLink = aliasPath;
246-
} else {
247-
nmLink = "";
248-
}
249-
} catch (e) {
250-
}
239+
nmLink = linkNodeModules(cwd, argv.preferLocal);
251240
}
252241
if (argv.install) {
253242
yield (0, import_deps.installDeps)((0, import_deps.parseDeps)(script), cwd, argv.registry);
@@ -264,9 +253,23 @@ function linkNodeModules(cwd, external) {
264253
const nm = "node_modules";
265254
const alias = import_index.path.resolve(cwd, nm);
266255
const target = import_index.path.basename(external) === nm ? import_index.path.resolve(external) : import_index.path.resolve(external, nm);
267-
if (import_index.fs.existsSync(alias) || !import_index.fs.existsSync(target)) return "";
256+
const aliasStat = lstat(alias);
257+
const targetStat = lstat(target);
258+
if (!(targetStat == null ? void 0 : targetStat.isDirectory()))
259+
throw new import_index.Fail(
260+
`Can't link node_modules: ${target} doesn't exist or is not a directory`
261+
);
262+
if ((aliasStat == null ? void 0 : aliasStat.isDirectory()) && alias !== target)
263+
throw new import_index.Fail(`Can't link node_modules: ${alias} already exists`);
264+
if (aliasStat) return "";
268265
import_index.fs.symlinkSync(target, alias, "junction");
269-
return target;
266+
return alias;
267+
}
268+
function lstat(p) {
269+
try {
270+
return import_index.fs.lstatSync(p);
271+
} catch (e) {
272+
}
270273
}
271274
function readScript() {
272275
return __async(this, null, function* () {

‎src/cli.ts‎

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -128,22 +128,19 @@ export async function main(): Promise<void> {
128128
await runScript(script, scriptPath, tempPath)
129129
}
130130

131-
// Short & safe remove: unlink symlinks; recurse only for real dirs/files
132131
const rmrf = (p: string) => {
133132
if (!p) return
134-
try {
135-
fs.lstatSync(p).isSymbolicLink()
136-
? fs.unlinkSync(p)
137-
: fs.rmSync(p, { force: true, recursive: true })
138-
} catch {}
139-
}
140133

134+
lstat(p)?.isSymbolicLink()
135+
? fs.unlinkSync(p)
136+
: fs.rmSync(p, { force: true, recursive: true })
137+
}
141138
async function runScript(
142139
script: string,
143140
scriptPath: string,
144141
tempPath: string
145142
): Promise<void> {
146-
let nmLink = '' // will hold the alias path (./node_modules) ONLY if it's a symlink
143+
let nmLink = ''
147144
const rmTemp = () => {
148145
rmrf(tempPath)
149146
rmrf(nmLink)
@@ -154,25 +151,9 @@ async function runScript(
154151
await fs.writeFile(tempPath, script)
155152
}
156153
const cwd = path.dirname(scriptPath)
157-
158154
if (typeof argv.preferLocal === 'string') {
159-
// Keep original behaviour: linkNodeModules returns TARGET (unchanged API)
160-
linkNodeModules(cwd, argv.preferLocal)
161-
162-
// For cleanup, compute ALIAS and only unlink if it's a symlink
163-
try {
164-
const aliasPath = path.resolve(cwd, 'node_modules')
165-
if (
166-
fs.existsSync(aliasPath) &&
167-
fs.lstatSync(aliasPath).isSymbolicLink()
168-
) {
169-
nmLink = aliasPath
170-
} else {
171-
nmLink = ''
172-
}
173-
} catch {}
155+
nmLink = linkNodeModules(cwd, argv.preferLocal)
174156
}
175-
176157
if (argv.install) {
177158
await installDeps(parseDeps(script), cwd, argv.registry)
178159
}
@@ -194,12 +175,25 @@ function linkNodeModules(cwd: string, external: string): string {
194175
path.basename(external) === nm
195176
? path.resolve(external)
196177
: path.resolve(external, nm)
178+
const aliasStat = lstat(alias)
179+
const targetStat = lstat(target)
197180

198-
if (fs.existsSync(alias) || !fs.existsSync(target)) return ''
181+
if (!targetStat?.isDirectory())
182+
throw new Fail(
183+
`Can't link node_modules: ${target} doesn't exist or is not a directory`
184+
)
185+
if (aliasStat?.isDirectory() && alias !== target)
186+
throw new Fail(`Can't link node_modules: ${alias} already exists`)
187+
if (aliasStat) return ''
199188

200189
fs.symlinkSync(target, alias, 'junction')
201-
// Keep behaviour stable: return TARGET (not alias)
202-
return target
190+
return alias
191+
}
192+
193+
function lstat(p: string) {
194+
try {
195+
return fs.lstatSync(p)
196+
} catch {}
203197
}
204198

205199
async function readScript() {

‎test/cli.test.js‎

Lines changed: 85 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -175,28 +175,99 @@ describe('cli', () => {
175175
}
176176
})
177177

178-
test('supports --prefer-local to load modules', async () => {
179-
const cwd = tmpdir()
180-
const external = tmpdir()
181-
await fs.outputJson(path.join(external, 'node_modules/a/package.json'), {
178+
describe('`--prefer-local`', () => {
179+
const pkgIndex = `export const a = 'AAA'`
180+
const pkgJson = {
182181
name: 'a',
183182
version: '1.0.0',
184183
type: 'module',
185184
exports: './index.js',
186-
})
187-
await fs.outputFile(
188-
path.join(external, 'node_modules/a/index.js'),
189-
`
190-
export const a = 'AAA'
191-
`
192-
)
185+
}
193186
const script = `
194187
import {a} from 'a'
195188
console.log(a);
196189
`
197-
const out =
198-
await $`node build/cli.js --cwd=${cwd} --prefer-local=${external} --test <<< ${script}`
199-
assert.equal(out.stdout, 'AAA\n')
190+
191+
test('true', async () => {
192+
const cwd = tmpdir()
193+
await fs.outputFile(path.join(cwd, 'node_modules/a/index.js'), pkgIndex)
194+
await fs.outputJson(
195+
path.join(cwd, 'node_modules/a/package.json'),
196+
pkgJson
197+
)
198+
199+
const out =
200+
await $`node build/cli.js --cwd=${cwd} --prefer-local=true --test <<< ${script}`
201+
assert.equal(out.stdout, 'AAA\n')
202+
assert.ok(await fs.exists(path.join(cwd, 'node_modules/a/index.js')))
203+
})
204+
205+
test('external dir', async () => {
206+
const cwd = tmpdir()
207+
const external = tmpdir()
208+
await fs.outputFile(
209+
path.join(external, 'node_modules/a/index.js'),
210+
pkgIndex
211+
)
212+
await fs.outputJson(
213+
path.join(external, 'node_modules/a/package.json'),
214+
pkgJson
215+
)
216+
217+
const out =
218+
await $`node build/cli.js --cwd=${cwd} --prefer-local=${external} --test <<< ${script}`
219+
assert.equal(out.stdout, 'AAA\n')
220+
assert.ok(await fs.exists(path.join(external, 'node_modules/a/index.js')))
221+
})
222+
223+
test('external alias', async () => {
224+
const cwd = tmpdir()
225+
const external = tmpdir()
226+
await fs.outputFile(
227+
path.join(external, 'node_modules/a/index.js'),
228+
pkgIndex
229+
)
230+
await fs.outputJson(
231+
path.join(external, 'node_modules/a/package.json'),
232+
pkgJson
233+
)
234+
await fs.symlinkSync(
235+
path.join(external, 'node_modules'),
236+
path.join(cwd, 'node_modules'),
237+
'junction'
238+
)
239+
240+
const out =
241+
await $`node build/cli.js --cwd=${cwd} --prefer-local=true --test <<< ${script}`
242+
assert.equal(out.stdout, 'AAA\n')
243+
assert.ok(await fs.exists(path.join(cwd, 'node_modules')))
244+
})
245+
246+
test('throws if exists', async () => {
247+
const cwd = tmpdir()
248+
const external = tmpdir()
249+
await fs.outputFile(path.join(cwd, 'node_modules/a/index.js'), pkgIndex)
250+
await fs.outputFile(
251+
path.join(external, 'node_modules/a/index.js'),
252+
pkgIndex
253+
)
254+
assert.rejects(
255+
() =>
256+
$`node build/cli.js --cwd=${cwd} --prefer-local=${external} --test <<< ${script}`,
257+
/node_modules already exists/
258+
)
259+
})
260+
261+
test('throws if not dir', async () => {
262+
const cwd = tmpdir()
263+
const external = tmpdir()
264+
await fs.outputFile(path.join(external, 'node_modules'), pkgIndex)
265+
assert.rejects(
266+
() =>
267+
$`node build/cli.js --cwd=${cwd} --prefer-local=${external} --test <<< ${script}`,
268+
/node_modules doesn't exist or is not a directory/
269+
)
270+
})
200271
})
201272

202273
test('scripts from https 200', async () => {

0 commit comments

Comments
 (0)