Skip to content

Commit 17c0ff2

Browse files
authored
fix(cli): more wizard improvements (#6769)
* fix(cli): more wizard improvements * chore:
1 parent 5768f9b commit 17c0ff2

8 files changed

Lines changed: 182 additions & 48 deletions

File tree

packages/cli/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
"@clack/prompts": "^1.5.1",
4747
"@stencil/dev-server": "workspace:*",
4848
"@stencil/templates": "workspace:*",
49+
"local-pkg": "^1.2.1",
4950
"nypm": "^0.3.12",
5051
"std-env": "^3.8.0",
5152
"typescript": "catalog:"

packages/cli/src/_test_/task-init.spec.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,18 @@ vi.mock('nypm', () => ({
2121
installDependencies: vi.fn().mockResolvedValue(undefined),
2222
detectPackageManager: vi.fn().mockResolvedValue({ name: 'npm' }),
2323
}));
24+
vi.mock('local-pkg', () => ({
25+
// installs "succeed" by default; tests override to simulate a package manager
26+
// that reports success but silently fails to write to node_modules
27+
getPackageInfo: vi.fn().mockResolvedValue({ rootPath: '/node_modules/pkg' }),
28+
}));
2429

2530
vi.mock('@clack/prompts', () => ({
2631
intro: vi.fn(),
2732
outro: vi.fn(),
2833
note: vi.fn(),
2934
confirm: vi.fn().mockResolvedValue(true),
30-
log: { warn: vi.fn(), info: vi.fn() },
35+
log: { warn: vi.fn(), info: vi.fn(), error: vi.fn() },
3136
spinner: vi.fn(() => ({ start: vi.fn(), stop: vi.fn() })),
3237
cancel: vi.fn(),
3338
isCancel: vi.fn().mockReturnValue(false),
@@ -83,6 +88,7 @@ import {
8388
generatePackageJsonFields,
8489
generateStencilConfig,
8590
} from '@stencil/templates';
91+
import { getPackageInfo } from 'local-pkg';
8692
import { addDevDependency, installDependencies } from 'nypm';
8793
import type { ValidatedConfig } from '@stencil/core/compiler';
8894

@@ -179,6 +185,7 @@ describe('taskInit', () => {
179185
vi.mocked(generateStencilConfig).mockReturnValue(null);
180186
vi.mocked(clack.confirm).mockResolvedValue(true);
181187
vi.mocked(clack.isCancel).mockReturnValue(false);
188+
vi.mocked(getPackageInfo).mockResolvedValue({ rootPath: '/node_modules/pkg' } as never);
182189
vi.spyOn(process, 'cwd').mockReturnValue(CWD);
183190
vi.spyOn(process, 'exit').mockImplementation((code) => {
184191
throw new Error(`exit:${code ?? 0}`);
@@ -338,6 +345,25 @@ describe('taskInit', () => {
338345
});
339346
});
340347

348+
it('fails loudly when the base install reports success but @stencil/core is not resolvable', async () => {
349+
vi.mocked(getPackageInfo).mockResolvedValue(undefined);
350+
await expect(taskInit(mockCoreCompiler, mockStrictConfig)).rejects.toThrow('exit:1');
351+
expect(clack.log.error).toHaveBeenCalledWith(expect.stringContaining('@stencil/core'));
352+
});
353+
354+
it('fails loudly when an integration reports installed but is not resolvable', async () => {
355+
vi.mocked(promptIntegrations).mockResolvedValue([makeIntegration('@stencil/vitest')]);
356+
// @stencil/core resolves fine, but the just-added integration does not
357+
vi.mocked(getPackageInfo).mockImplementation((name: string) =>
358+
Promise.resolve(name === '@stencil/core' ? { rootPath: '/node_modules/pkg' } : undefined),
359+
);
360+
361+
await expect(taskInit(mockCoreCompiler, mockStrictConfig)).rejects.toThrow('exit:1');
362+
expect(clack.log.error).toHaveBeenCalledWith(expect.stringContaining('@stencil/vitest'));
363+
// never reaches plugin discovery/run since verification fails first
364+
expect(vi.mocked(discoverPlugins)).not.toHaveBeenCalled();
365+
});
366+
341367
it('does not discover plugins when no integrations are selected', async () => {
342368
await taskInit(mockCoreCompiler, mockStrictConfig);
343369
expect(vi.mocked(discoverPlugins)).not.toHaveBeenCalled();
@@ -488,6 +514,15 @@ describe('taskInit', () => {
488514
});
489515
});
490516

517+
it('fails loudly when a newly installed integration is not actually resolvable', async () => {
518+
const vitest = makeIntegration('@stencil/vitest');
519+
vi.mocked(promptAddCapabilities).mockResolvedValue({ toInstall: [vitest], toConfigure: [] });
520+
vi.mocked(getPackageInfo).mockResolvedValue(undefined);
521+
522+
await expect(taskInit(mockCoreCompiler, mockStrictConfig)).rejects.toThrow('exit:1');
523+
expect(clack.log.error).toHaveBeenCalledWith(expect.stringContaining('@stencil/vitest'));
524+
});
525+
491526
it('calls run() for newly installed packages after re-discovery', async () => {
492527
const vitest = makeIntegration('@stencil/vitest');
493528
vi.mocked(promptAddCapabilities).mockResolvedValue({ toInstall: [vitest], toConfigure: [] });

packages/cli/src/_test_/wizard-discover.spec.ts

Lines changed: 69 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,12 @@ import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest';
33
import { discoverPlugins } from '../wizard/discover';
44

55
vi.mock('node:fs/promises', () => ({ readFile: vi.fn() }));
6+
vi.mock('local-pkg', () => ({ getPackageInfo: vi.fn() }));
67

78
import { readFile } from 'node:fs/promises';
9+
import { getPackageInfo } from 'local-pkg';
810
const mockReadFile = readFile as ReturnType<typeof vi.fn>;
11+
const mockGetPackageInfo = getPackageInfo as ReturnType<typeof vi.fn>;
912

1013
const ROOT = '/project';
1114

@@ -16,8 +19,19 @@ function makeRootPkg(
1619
return JSON.stringify({ dependencies: deps, devDependencies: devDeps });
1720
}
1821

19-
function makeDepPkg(wizardEntry?: string): string {
20-
return JSON.stringify(wizardEntry ? { stencil: { wizard: wizardEntry } } : {});
22+
/** Registers a resolvable package at `${ROOT}/node_modules/${packageName}`, optionally declaring a wizard entry. */
23+
function mockResolvedPackage(packageName: string, wizardEntry?: string) {
24+
const rootPath = `${ROOT}/node_modules/${packageName}`;
25+
mockGetPackageInfo.mockImplementation((name: string) => {
26+
if (name !== packageName) return Promise.resolve(undefined);
27+
return Promise.resolve({
28+
name,
29+
version: '1.0.0',
30+
rootPath,
31+
packageJsonPath: `${rootPath}/package.json`,
32+
packageJson: wizardEntry ? { stencil: { wizard: wizardEntry } } : {},
33+
});
34+
});
2135
}
2236

2337
function makeLoader(modules: Record<string, Record<string, unknown>> = {}) {
@@ -34,6 +48,7 @@ describe('discoverPlugins', () => {
3448

3549
beforeEach(() => {
3650
warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
51+
mockGetPackageInfo.mockReset().mockResolvedValue(undefined);
3752
});
3853

3954
afterEach(() => {
@@ -47,36 +62,54 @@ describe('discoverPlugins', () => {
4762
});
4863

4964
it('returns empty array when no deps declare stencil.wizard', async () => {
50-
mockReadFile
51-
.mockResolvedValueOnce(makeRootPkg({ '@stencil/vitest': '^1.0.0' }))
52-
.mockResolvedValueOnce(makeDepPkg(/* no wizard */));
65+
mockReadFile.mockResolvedValueOnce(makeRootPkg({ '@stencil/vitest': '^1.0.0' }));
66+
mockResolvedPackage('@stencil/vitest' /* no wizard */);
67+
68+
const result = await discoverPlugins(ROOT, makeLoader());
69+
expect(result).toEqual([]);
70+
});
71+
72+
it('returns empty array when a dep cannot be resolved at all', async () => {
73+
mockReadFile.mockResolvedValueOnce(makeRootPkg({ '@stencil/vitest': '^1.0.0' }));
74+
// mockGetPackageInfo already defaults to resolving undefined
5375

5476
const result = await discoverPlugins(ROOT, makeLoader());
5577
expect(result).toEqual([]);
5678
});
5779

58-
it('discovers a plugin from dependencies', async () => {
80+
it('discovers a plugin from dependencies, resolved via node module resolution', async () => {
5981
const plugin = { generate: { fileTemplates: [] } };
60-
mockReadFile
61-
.mockResolvedValueOnce(makeRootPkg({ '@stencil/vitest': '^1.0.0' }))
62-
.mockResolvedValueOnce(makeDepPkg('./dist/wizard.js'));
82+
mockReadFile.mockResolvedValueOnce(makeRootPkg({ '@stencil/vitest': '^1.0.0' }));
83+
mockResolvedPackage('@stencil/vitest', './dist/wizard.js');
6384

6485
const wizardPath = `${ROOT}/node_modules/@stencil/vitest/dist/wizard.js`;
6586
const loader = makeLoader({ [wizardPath]: { wizard: plugin } });
6687

6788
const result = await discoverPlugins(ROOT, loader);
6889
expect(result).toEqual([{ packageName: '@stencil/vitest', plugin }]);
90+
// rootDir is passed through as a resolution path, so hoisted deps (workspace root
91+
// node_modules) resolve correctly, not just a literal rootDir/node_modules join
92+
expect(mockGetPackageInfo).toHaveBeenCalledWith('@stencil/vitest', { paths: [ROOT] });
6993
});
7094

7195
it('discovers plugins from both dependencies and devDependencies', async () => {
7296
const pluginA = { generate: { fileTemplates: [] } };
7397
const pluginB = { init: { id: 'sass', displayName: 'Sass', description: '' } };
74-
mockReadFile
75-
.mockResolvedValueOnce(
76-
makeRootPkg({ '@stencil/vitest': '^1.0.0' }, { '@stencil/sass': '^3.0.0' }),
77-
)
78-
.mockResolvedValueOnce(makeDepPkg('./dist/wizard.js')) // @stencil/vitest
79-
.mockResolvedValueOnce(makeDepPkg('./dist/wizard.js')); // @stencil/sass
98+
mockReadFile.mockResolvedValueOnce(
99+
makeRootPkg({ '@stencil/vitest': '^1.0.0' }, { '@stencil/sass': '^3.0.0' }),
100+
);
101+
mockGetPackageInfo.mockImplementation((name: string) => {
102+
const wizardEntry = './dist/wizard.js';
103+
if (name !== '@stencil/vitest' && name !== '@stencil/sass') return Promise.resolve(undefined);
104+
const rootPath = `${ROOT}/node_modules/${name}`;
105+
return Promise.resolve({
106+
name,
107+
version: '1.0.0',
108+
rootPath,
109+
packageJsonPath: `${rootPath}/package.json`,
110+
packageJson: { stencil: { wizard: wizardEntry } },
111+
});
112+
});
80113

81114
const loader = makeLoader({
82115
[`${ROOT}/node_modules/@stencil/vitest/dist/wizard.js`]: { wizard: pluginA },
@@ -90,9 +123,8 @@ describe('discoverPlugins', () => {
90123
});
91124

92125
it('skips a plugin whose module fails to load and warns', async () => {
93-
mockReadFile
94-
.mockResolvedValueOnce(makeRootPkg({ '@stencil/vitest': '^1.0.0' }))
95-
.mockResolvedValueOnce(makeDepPkg('./dist/wizard.js'));
126+
mockReadFile.mockResolvedValueOnce(makeRootPkg({ '@stencil/vitest': '^1.0.0' }));
127+
mockResolvedPackage('@stencil/vitest', './dist/wizard.js');
96128

97129
const loader = makeLoader(/* no matching module */);
98130

@@ -103,9 +135,8 @@ describe('discoverPlugins', () => {
103135
});
104136

105137
it('skips a plugin that does not export wizard and warns', async () => {
106-
mockReadFile
107-
.mockResolvedValueOnce(makeRootPkg({ '@stencil/vitest': '^1.0.0' }))
108-
.mockResolvedValueOnce(makeDepPkg('./dist/wizard.js'));
138+
mockReadFile.mockResolvedValueOnce(makeRootPkg({ '@stencil/vitest': '^1.0.0' }));
139+
mockResolvedPackage('@stencil/vitest', './dist/wizard.js');
109140

110141
const wizardPath = `${ROOT}/node_modules/@stencil/vitest/dist/wizard.js`;
111142
const loader = makeLoader({ [wizardPath]: { notWizard: {} } });
@@ -119,12 +150,21 @@ describe('discoverPlugins', () => {
119150

120151
it('does not fail when one plugin errors and others succeed', async () => {
121152
const plugin = { generate: { fileTemplates: [] } };
122-
mockReadFile
123-
.mockResolvedValueOnce(
124-
makeRootPkg({ '@stencil/vitest': '^1.0.0', '@stencil/broken': '^1.0.0' }),
125-
)
126-
.mockResolvedValueOnce(makeDepPkg('./dist/wizard.js')) // @stencil/vitest
127-
.mockResolvedValueOnce(makeDepPkg('./dist/wizard.js')); // @stencil/broken
153+
mockReadFile.mockResolvedValueOnce(
154+
makeRootPkg({ '@stencil/vitest': '^1.0.0', '@stencil/broken': '^1.0.0' }),
155+
);
156+
mockGetPackageInfo.mockImplementation((name: string) => {
157+
if (name !== '@stencil/vitest' && name !== '@stencil/broken')
158+
return Promise.resolve(undefined);
159+
const rootPath = `${ROOT}/node_modules/${name}`;
160+
return Promise.resolve({
161+
name,
162+
version: '1.0.0',
163+
rootPath,
164+
packageJsonPath: `${rootPath}/package.json`,
165+
packageJson: { stencil: { wizard: './dist/wizard.js' } },
166+
});
167+
});
128168

129169
const loader = makeLoader({
130170
[`${ROOT}/node_modules/@stencil/vitest/dist/wizard.js`]: { wizard: plugin },
@@ -166,9 +206,9 @@ describe('discoverPlugins', () => {
166206
const installedPlugin = { generate: { fileTemplates: [] } };
167207
mockReadFile
168208
.mockResolvedValueOnce(makeRootPkg({ '@stencil/sass': '^3.0.0' }))
169-
.mockResolvedValueOnce(makeDepPkg('./dist/wizard.js')) // @stencil/sass
170209
.mockRejectedValueOnce(new Error('ENOENT')) // dist/package.json not found
171210
.mockResolvedValueOnce(JSON.stringify({ name: 'my-plugin' })); // parent package.json
211+
mockResolvedPackage('@stencil/sass', './dist/wizard.js');
172212

173213
const loader = makeLoader({
174214
[`${ROOT}/node_modules/@stencil/sass/dist/wizard.js`]: { wizard: installedPlugin },
@@ -186,9 +226,9 @@ describe('discoverPlugins', () => {
186226
const installedPlugin = { generate: { fileTemplates: [] } };
187227
mockReadFile
188228
.mockResolvedValueOnce(makeRootPkg({ 'my-plugin': '^1.0.0' }))
189-
.mockResolvedValueOnce(makeDepPkg('./dist/wizard.js')) // installed my-plugin
190229
.mockRejectedValueOnce(new Error('ENOENT')) // dev: dist/package.json not found
191230
.mockResolvedValueOnce(JSON.stringify({ name: 'my-plugin' })); // dev: parent package.json
231+
mockResolvedPackage('my-plugin', './dist/wizard.js');
192232

193233
const loader = makeLoader({
194234
[`${ROOT}/node_modules/my-plugin/dist/wizard.js`]: { wizard: installedPlugin },

packages/cli/src/task-init.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
generateStencilConfig,
88
toPascalCase,
99
} from '@stencil/templates';
10+
import { getPackageInfo } from 'local-pkg';
1011
import * as nypm from 'nypm';
1112
import { addDevDependency, installDependencies } from 'nypm';
1213
import { isCI } from 'std-env';
@@ -49,6 +50,31 @@ import {
4950
import { printSplash } from './wizard/splash.js';
5051
import type { CoreCompiler } from './load-compiler.js';
5152

53+
/**
54+
* Package managers can report success while silently failing to write to node_modules
55+
* (observed with npm computing a resolved lockfile but reifying zero filesystem changes).
56+
* Confirms packages actually landed on disk instead of trusting the install call's exit code.
57+
*
58+
* @param rootDir - directory to resolve packages from.
59+
* @param packageNames - packages that were just supposed to be installed.
60+
* @returns names that failed to resolve; empty when everything installed correctly.
61+
*/
62+
async function verifyInstalled(rootDir: string, packageNames: string[]): Promise<string[]> {
63+
const missing: string[] = [];
64+
for (const name of packageNames) {
65+
const info = await getPackageInfo(name, { paths: [rootDir] });
66+
if (!info) missing.push(name);
67+
}
68+
return missing;
69+
}
70+
71+
function failInstallVerification(missing: string[], rootDir: string): never {
72+
p.log.error(
73+
`Install reported success but ${missing.join(', ')} could not be found in ${rootDir}. Your package manager may have silently failed to write to node_modules - check its logs and try again.`,
74+
);
75+
process.exit(1);
76+
}
77+
5278
function outputKeysToTargets(keys: ReadonlyArray<OutputKey>): Array<{ type: string }> {
5379
const map: Record<OutputKey, string> = {
5480
loader: 'loader-bundle',
@@ -153,13 +179,19 @@ export async function taskInit(
153179
await installDependencies({ cwd, silent: true });
154180
s2.stop('Dependencies installed');
155181

182+
const missingBase = await verifyInstalled(coreDir, ['@stencil/core']);
183+
if (missingBase.length > 0) failInstallVerification(missingBase, coreDir);
184+
156185
if (selectedIntegrations.length > 0) {
157186
const s3 = p.spinner();
158187
const pkgs = selectedIntegrations.map((i) => i.package);
159188
s3.start(`Installing ${pkgs.join(', ')}`);
160189
// Integration packages (e.g. output target plugins) are deps of the core package
161190
await addDevDependency(withVersionRanges(pkgs), { cwd: coreDir, silent: true });
162191
s3.stop('Integrations installed');
192+
193+
const missingIntegrations = await verifyInstalled(coreDir, pkgs);
194+
if (missingIntegrations.length > 0) failInstallVerification(missingIntegrations, coreDir);
163195
}
164196

165197
// Phase 4: re-discover + run plugin wizards
@@ -263,6 +295,9 @@ async function addCapabilities(cwd: string, strictConfig?: ValidatedConfig): Pro
263295
s.start(`Installing ${pkgs.join(', ')}`);
264296
await addDevDependency(withVersionRanges(pkgs), { cwd, silent: true });
265297
s.stop('Dependencies installed');
298+
299+
const missing = await verifyInstalled(cwd, pkgs);
300+
if (missing.length > 0) failInstallVerification(missing, cwd);
266301
}
267302

268303
// Re-discover after install so newly installed packages can run their wizards

packages/cli/src/wizard/discover.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { readFile } from 'node:fs/promises';
22
import { basename, dirname, join, resolve } from 'node:path';
33
import { pathToFileURL } from 'node:url';
4+
import { getPackageInfo } from 'local-pkg';
45

56
import type { StencilWizardPlugin } from './types.js';
67

@@ -24,11 +25,14 @@ async function readJson(filePath: string) {
2425
}
2526

2627
async function loadOne(rootDir: string, packageName: string, loader: ModuleLoader) {
27-
const depPkg = await readJson(join(rootDir, 'node_modules', packageName, 'package.json'));
28-
const wizardEntry = (depPkg?.stencil as { wizard?: string } | undefined)?.wizard;
28+
// resolved via node's module resolution (not a plain rootDir/node_modules join) so
29+
// hoisted deps in npm/pnpm/yarn workspaces are found regardless of where they land
30+
const info = await getPackageInfo(packageName, { paths: [rootDir] });
31+
if (!info) return null;
32+
const wizardEntry = (info.packageJson.stencil as { wizard?: string } | undefined)?.wizard;
2933
if (!wizardEntry) return null;
3034

31-
const wizardPath = join(rootDir, 'node_modules', packageName, wizardEntry);
35+
const wizardPath = join(info.rootPath, wizardEntry);
3236
let mod: Record<string, unknown>;
3337
try {
3438
mod = await loader(pathToFileURL(wizardPath).href);

0 commit comments

Comments
 (0)