Skip to content

fix(plugin): install monorepo sub-plugin deps when not hoisted #1007

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jackwener merged 1 commit into from
Apr 14, 2026
Merged

fix(plugin): install monorepo sub-plugin deps when not hoisted #1007

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion src/plugin.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ describe('postInstallMonorepoLifecycle', () => {
fs.rmSync(repoDir, { recursive: true, force: true });
});

it('installs dependencies once at the monorepo root, not in each sub-plugin', () => {
it('installs dependencies at the monorepo root and skips sub-plugins without own dependencies', () => {
_postInstallMonorepoLifecycle(repoDir, [subDir]);

const npmCalls = mockExecFileSync.mock.calls.filter(
Expand All @@ -650,6 +650,26 @@ describe('postInstallMonorepoLifecycle', () => {
expect(npmCalls[0][2]).toMatchObject({ cwd: repoDir });
expect(npmCalls.some(([, , opts]) => opts?.cwd === subDir)).toBe(false);
});
Comment on lines 642 to 652
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "skips sub-plugins without own dependencies" test currently doesn't create a package.json in subDir, so it can pass because the file is missing rather than because hasOwnDependencies correctly returns false for an existing package.json with empty/missing dependencies. To cover the intended behavior, write a minimal package.json for subDir that has no dependencies (or an empty object) and assert that no npm install runs in subDir.

Copilot uses AI. Check for mistakes.

it('also installs dependencies in sub-plugins that declare their own production dependencies', () => {
// Give the sub-plugin its own production dependencies
fs.writeFileSync(path.join(subDir, 'package.json'), JSON.stringify({
name: 'opencli-plugin-alpha',
version: '1.0.0',
type: 'module',
dependencies: { undici: '^8.0.0' },
}));

_postInstallMonorepoLifecycle(repoDir, [subDir]);

const npmCalls = mockExecFileSync.mock.calls.filter(
([cmd, args]) => cmd === 'npm' && Array.isArray(args) && args[0] === 'install',
);

expect(npmCalls).toHaveLength(2);
expect(npmCalls[0][2]).toMatchObject({ cwd: repoDir });
expect(npmCalls[1][2]).toMatchObject({ cwd: subDir });
});
});

describe('updateAllPlugins', () => {
Expand Down
23 changes: 22 additions & 1 deletion src/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,18 @@ export function validatePluginStructure(pluginDir: string): ValidationResult {
return { valid: errors.length === 0, errors };
}

/** Check whether a directory has its own production dependencies in package.json. */
function hasOwnDependencies(dir: string): boolean {
const pkgPath = path.join(dir, 'package.json');
if (!fs.existsSync(pkgPath)) return false;
try {
const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf-8'));
return pkg.dependencies != null && Object.keys(pkg.dependencies).length > 0;
} catch {
return false;
Comment on lines +583 to +584
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasOwnDependencies swallows JSON parse/read errors and returns false, which can cause sub-plugin installs to silently skip npm install even when a package.json exists but is malformed. That can lead to a successful install followed by a runtime "Cannot find package" error instead of failing early with a clear message. Consider surfacing a PluginError for invalid package.json, or conservatively treating parse failures as "has deps" so installDependencies runs and produces an actionable failure.

Suggested change
} catch {
return false;
} catch (err) {
throw new PluginError(
`Invalid package.json in ${dir}: ${getErrorMessage(err)}`,
'Fix the plugin package.json so dependencies can be determined and installed correctly.',
);

Copilot uses AI. Check for mistakes.
}
}

function installDependencies(dir: string): void {
const pkgJsonPath = path.join(dir, 'package.json');
if (!fs.existsSync(pkgJsonPath)) return;
Expand Down Expand Up @@ -607,11 +619,20 @@ function postInstallLifecycle(pluginDir: string): void {
}

/**
* Monorepo lifecycle: install shared deps once at repo root, then finalize each sub-plugin.
* Monorepo lifecycle: install shared deps at repo root, then install and finalize each sub-plugin.
*
* The root install covers monorepos that use npm workspaces to hoist dependencies.
* For monorepos that do NOT use workspaces, sub-plugins may declare their own
* production dependencies in their package.json. We install those per sub-plugin
* so that runtime imports (e.g. `undici`) can be resolved from the sub-plugin
* directory. When the root already satisfies all deps this is a fast no-op.
*/
function postInstallMonorepoLifecycle(repoDir: string, pluginDirs: string[]): void {
installDependencies(repoDir);
for (const pluginDir of pluginDirs) {
if (pluginDir !== repoDir && hasOwnDependencies(pluginDir)) {
installDependencies(pluginDir);
}
finalizePluginRuntime(pluginDir);
Comment on lines 630 to 636
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will run npm install --omit=dev inside every sub-plugin that has a non-empty dependencies object. In large monorepos this can multiply install time and may repeatedly touch the workspace/root lockfile even when the root install already covers dependency resolution (e.g., npm workspaces/hoisting). Consider detecting a workspace/hoisting setup at the repo root and skipping per-sub-plugin installs in that case, or otherwise limiting redundant installs (e.g., only when the root has no workspaces and the sub-plugin lacks a usable node_modules).

Copilot uses AI. Check for mistakes.
}
}
Expand Down