Skip to content

Commit f6529d3

Browse files
ntottenclaude
andcommitted
Fix pnpm module resolution and web test compilation
- Fix web extension test by using test-compile instead of just webpack (TypeScript needs to be compiled before running web tests) - Add findPkgThroughTransitiveDeps to ModuleResolver to support pnpm's strict node_modules structure (non-hoisted dependencies) - Pin prettier version in module-dep test fixture to 2.0.2 for test consistency The new module resolution logic: 1. First checks for explicit prettier dependency in package.json 2. Then checks transitive dependencies by looking at installed packages in node_modules and resolving prettier from within each dependency 3. Falls back to looking for node_modules/prettier directly This ensures the extension works correctly with npm, yarn, and pnpm regardless of their dependency hoisting behavior. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent e214794 commit f6529d3

File tree

4 files changed

+139
-8
lines changed

4 files changed

+139
-8
lines changed

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@
8282
"webpack-dev": "webpack --mode development --watch",
8383
"webpack": "webpack --mode development",
8484
"chrome": "pnpm webpack && vscode-test-web --browserType=chromium --extensionDevelopmentPath=. .",
85-
"test:web": "pnpm webpack && node ./out/test/web/runTests.js",
85+
"test:web": "pnpm test-compile && node ./out/test/web/runTests.js",
8686
"prepare": "husky"
8787
},
8888
"lint-staged": {

src/ModuleResolver.ts

Lines changed: 132 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,123 @@ export class ModuleResolver implements ModuleResolverInterface {
465465
}
466466
}
467467

468+
/**
469+
* Find a package through transitive dependencies. This is necessary for
470+
* package managers like pnpm that don't hoist dependencies to the root
471+
* node_modules. For example, if project A depends on package B, and B
472+
* depends on prettier, pnpm won't make prettier accessible from A's directory.
473+
* This method checks each dependency to see if it provides access to the target package.
474+
*
475+
* @param {string} startDir Directory to start searching from
476+
* @param {string} pkgName Package name to search for
477+
* @returns {string | undefined} Resolved path to the package, or undefined if not found
478+
*/
479+
private findPkgThroughTransitiveDeps(
480+
startDir: string,
481+
pkgName: string,
482+
): string | undefined {
483+
const searchResult = findUp.sync(
484+
(dir) => {
485+
const nodeModulesPath = path.join(dir, "node_modules");
486+
if (!fs.existsSync(nodeModulesPath)) {
487+
if (this.isInternalTestRoot(dir)) {
488+
return findUp.stop;
489+
}
490+
return;
491+
}
492+
493+
// Get list of installed dependencies from node_modules
494+
let installedDeps: string[];
495+
try {
496+
installedDeps = fs.readdirSync(nodeModulesPath).filter((name) => {
497+
// Skip hidden files and pnpm internal directories
498+
if (name.startsWith(".")) return false;
499+
// Check if it's a directory or symlink
500+
const depPath = path.join(nodeModulesPath, name);
501+
try {
502+
const stat = fs.lstatSync(depPath);
503+
return stat.isDirectory() || stat.isSymbolicLink();
504+
} catch {
505+
return false;
506+
}
507+
});
508+
} catch {
509+
if (this.isInternalTestRoot(dir)) {
510+
return findUp.stop;
511+
}
512+
return;
513+
}
514+
515+
// Check each dependency to see if it provides access to the target package
516+
for (const depName of installedDeps) {
517+
// Skip scoped packages for now (would need special handling)
518+
if (depName.startsWith("@")) continue;
519+
520+
const depPath = path.join(nodeModulesPath, depName);
521+
try {
522+
// Get the real path (resolves symlinks)
523+
const realDepPath = fs.realpathSync(depPath);
524+
525+
// Try to resolve the target package from within the dependency's directory
526+
try {
527+
resolve.sync(pkgName, {
528+
basedir: realDepPath,
529+
preserveSymlinks: false,
530+
});
531+
// Found! Return this directory
532+
return dir;
533+
} catch {
534+
// This dependency doesn't provide access to the target package
535+
}
536+
} catch {
537+
// Couldn't get real path for this dependency, skip it
538+
}
539+
}
540+
541+
if (this.isInternalTestRoot(dir)) {
542+
return findUp.stop;
543+
}
544+
},
545+
{ cwd: startDir, type: "directory" },
546+
);
547+
548+
if (searchResult) {
549+
// Now we need to find the actual path through the dependency
550+
const nodeModulesPath = path.join(searchResult, "node_modules");
551+
const installedDeps = fs.readdirSync(nodeModulesPath).filter((name) => {
552+
if (name.startsWith(".")) return false;
553+
const depPath = path.join(nodeModulesPath, name);
554+
try {
555+
const stat = fs.lstatSync(depPath);
556+
return stat.isDirectory() || stat.isSymbolicLink();
557+
} catch {
558+
return false;
559+
}
560+
});
561+
562+
for (const depName of installedDeps) {
563+
if (depName.startsWith("@")) continue;
564+
565+
const depPath = path.join(nodeModulesPath, depName);
566+
try {
567+
const realDepPath = fs.realpathSync(depPath);
568+
try {
569+
return resolve.sync(pkgName, {
570+
basedir: realDepPath,
571+
preserveSymlinks: false,
572+
});
573+
} catch {
574+
// Continue to next dependency
575+
}
576+
} catch {
577+
// Continue to next dependency
578+
}
579+
}
580+
}
581+
582+
return undefined;
583+
}
584+
468585
/**
469586
* Recursively search upwards for a given module definition based on
470587
* package.json or node_modules existence
@@ -524,7 +641,21 @@ export class ModuleResolver implements ModuleResolverInterface {
524641
return packagePath;
525642
}
526643

527-
// If no explicit package.json dep found, instead look for implicit dep
644+
// If no explicit package.json dep found, try to resolve through transitive
645+
// dependencies. This handles cases where prettier is a dependency of one of the
646+
// project's dependencies. This is important for pnpm which doesn't hoist
647+
// dependencies to the root node_modules.
648+
const transitiveResult = this.findPkgThroughTransitiveDeps(
649+
finalPath,
650+
pkgName,
651+
);
652+
653+
if (transitiveResult) {
654+
this.findPkgCache.set(cacheKey, transitiveResult);
655+
return transitiveResult;
656+
}
657+
658+
// If no transitive dep found, instead look for implicit dep in node_modules
528659
const nodeModulesResDir = findUp.sync(
529660
(dir) => {
530661
if (fs.existsSync(path.join(dir, "node_modules", pkgName))) {

test-fixtures/module-dep/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
"author": "",
1111
"license": "MIT",
1212
"dependencies": {
13-
"prettier": "^2.0.2"
13+
"prettier": "2.0.2"
1414
},
1515
"packageManager": "pnpm@10.24.0"
1616
}

test-fixtures/module-dep/pnpm-lock.yaml

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)