Skip to content

Commit d73c129

Browse files
valentinpalkovicstorybook-bot
valentinpalkovic
authored and
storybook-bot
committed
Merge pull request #32108 from gingeekrishna/fix-storybook-webpack-32105
Angular: Inherit options from browserTarget (cherry picked from commit b0f9e0d)
1 parent e10f871 commit d73c129

File tree

2 files changed

+343
-10
lines changed

2 files changed

+343
-10
lines changed

‎code/frameworks/angular/src/server/framework-preset-angular-cli.test.ts

Lines changed: 299 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,299 @@
1+
import { vi, expect, describe, it, beforeEach } from 'vitest';
2+
3+
import { logger } from 'storybook/internal/node-logger';
4+
5+
import type { BuilderContext } from '@angular-devkit/architect';
6+
import { logging } from '@angular-devkit/core';
7+
8+
import { getBuilderOptions } from './framework-preset-angular-cli';
9+
import type { PresetOptions } from './preset-options';
10+
11+
// Mock all dependencies
12+
vi.mock('storybook/internal/node-logger', () => ({
13+
logger: {
14+
info: vi.fn(),
15+
},
16+
}));
17+
18+
vi.mock('storybook/internal/server-errors', () => ({
19+
AngularLegacyBuildOptionsError: class AngularLegacyBuildOptionsError extends Error {
20+
constructor() {
21+
super('AngularLegacyBuildOptionsError');
22+
this.name = 'AngularLegacyBuildOptionsError';
23+
}
24+
},
25+
}));
26+
27+
vi.mock('storybook/internal/common', () => ({
28+
getProjectRoot: vi.fn(),
29+
}));
30+
31+
vi.mock('@angular-devkit/architect', () => ({
32+
targetFromTargetString: vi.fn(),
33+
}));
34+
35+
vi.mock('find-up', () => ({
36+
findUp: vi.fn(),
37+
}));
38+
39+
vi.mock('./utils/module-is-available', () => ({
40+
moduleIsAvailable: vi.fn(),
41+
}));
42+
43+
vi.mock('./angular-cli-webpack', () => ({
44+
getWebpackConfig: vi.fn(),
45+
}));
46+
47+
vi.mock('./preset-options', () => ({
48+
PresetOptions: {},
49+
}));
50+
51+
// Mock require.resolve for @angular/animations
52+
vi.mock('@angular/animations', () => ({}));
53+
54+
const mockedLogger = vi.mocked(logger);
55+
56+
const mockedTargetFromTargetString = vi.mocked(
57+
await import('@angular-devkit/architect')
58+
).targetFromTargetString;
59+
const mockedFindUp = vi.mocked(await import('find-up')).findUp;
60+
const mockedGetProjectRoot = vi.mocked(await import('storybook/internal/common')).getProjectRoot;
61+
62+
describe('framework-preset-angular-cli', () => {
63+
beforeEach(() => {
64+
vi.clearAllMocks();
65+
});
66+
67+
describe('getBuilderOptions', () => {
68+
const mockBuilderContext: BuilderContext = {
69+
target: { project: 'test-project', builder: 'test-builder', options: {} },
70+
workspaceRoot: '/test/workspace',
71+
getProjectMetadata: vi.fn().mockResolvedValue({}),
72+
getTargetOptions: vi.fn().mockResolvedValue({}),
73+
logger: new logging.Logger('Test'),
74+
} as unknown as BuilderContext;
75+
76+
beforeEach(() => {
77+
mockedGetProjectRoot.mockReturnValue('/test/project');
78+
mockedFindUp.mockResolvedValue('/test/tsconfig.json');
79+
});
80+
81+
it('should get browser target options when angularBrowserTarget is provided', async () => {
82+
const mockTarget = { project: 'test-project', target: 'build', configuration: 'development' };
83+
mockedTargetFromTargetString.mockReturnValue(mockTarget);
84+
85+
const options: PresetOptions = {
86+
configType: 'DEVELOPMENT',
87+
configDir: '/test/config',
88+
presets: {
89+
apply: vi.fn(),
90+
} as any,
91+
angularBrowserTarget: 'test-project:build:development',
92+
};
93+
94+
await getBuilderOptions(options, mockBuilderContext);
95+
96+
expect(mockedTargetFromTargetString).toHaveBeenCalledWith('test-project:build:development');
97+
expect(mockedLogger.info).toHaveBeenCalledWith(
98+
'=> Using angular browser target options from "test-project:build:development"'
99+
);
100+
expect(mockBuilderContext.getTargetOptions).toHaveBeenCalledWith(mockTarget);
101+
});
102+
103+
it('should merge browser target options with storybook options', async () => {
104+
const mockTarget = { project: 'test-project', target: 'build' };
105+
mockedTargetFromTargetString.mockReturnValue(mockTarget);
106+
107+
const browserTargetOptions = { a: 1, nested: { x: 10 } };
108+
const storybookOptions = { b: 2, nested: { y: 20 } };
109+
110+
vi.mocked(mockBuilderContext.getTargetOptions).mockResolvedValue(browserTargetOptions);
111+
112+
const options: PresetOptions = {
113+
configType: 'DEVELOPMENT',
114+
configDir: '/test/config',
115+
presets: {
116+
apply: vi.fn(),
117+
} as any,
118+
angularBrowserTarget: 'test-project:build',
119+
angularBuilderOptions: storybookOptions,
120+
};
121+
122+
const result = await getBuilderOptions(options, mockBuilderContext);
123+
124+
expect(result).toEqual({
125+
a: 1,
126+
b: 2,
127+
nested: { x: 10, y: 20 },
128+
tsConfig: '/test/tsconfig.json',
129+
});
130+
});
131+
132+
it('should use provided tsConfig when available', async () => {
133+
const options: PresetOptions = {
134+
configType: 'DEVELOPMENT',
135+
configDir: '/test/config',
136+
presets: {
137+
apply: vi.fn(),
138+
} as any,
139+
tsConfig: '/custom/tsconfig.json',
140+
};
141+
142+
const result = await getBuilderOptions(options, mockBuilderContext);
143+
144+
expect(result.tsConfig).toBe('/custom/tsconfig.json');
145+
expect(mockedLogger.info).toHaveBeenCalledWith(
146+
'=> Using angular project with "tsConfig:/custom/tsconfig.json"'
147+
);
148+
});
149+
150+
it('should find tsconfig.json when not provided', async () => {
151+
const options: PresetOptions = {
152+
configType: 'DEVELOPMENT',
153+
configDir: '/test/config',
154+
presets: {
155+
apply: vi.fn(),
156+
} as any,
157+
};
158+
159+
const result = await getBuilderOptions(options, mockBuilderContext);
160+
161+
expect(mockedFindUp).toHaveBeenCalledWith('tsconfig.json', {
162+
cwd: '/test/config',
163+
stopAt: '/test/project',
164+
});
165+
expect(result.tsConfig).toBe('/test/tsconfig.json');
166+
});
167+
168+
it('should use browser target tsConfig when no other tsConfig is available', async () => {
169+
const mockTarget = { project: 'test-project', target: 'build' };
170+
mockedTargetFromTargetString.mockReturnValue(mockTarget);
171+
mockedFindUp.mockResolvedValue(null);
172+
173+
const browserTargetOptions = { tsConfig: '/browser/tsconfig.json' };
174+
vi.mocked(mockBuilderContext.getTargetOptions).mockResolvedValue(browserTargetOptions);
175+
176+
const options: PresetOptions = {
177+
configType: 'DEVELOPMENT',
178+
configDir: '/test/config',
179+
presets: {
180+
apply: vi.fn(),
181+
} as any,
182+
angularBrowserTarget: 'test-project:build',
183+
};
184+
185+
const result = await getBuilderOptions(options, mockBuilderContext);
186+
187+
expect(result.tsConfig).toBe('/browser/tsconfig.json');
188+
});
189+
190+
it('should handle case when no angularBrowserTarget is provided', async () => {
191+
const options: PresetOptions = {
192+
configType: 'DEVELOPMENT',
193+
configDir: '/test/config',
194+
presets: {
195+
apply: vi.fn(),
196+
} as any,
197+
};
198+
199+
const result = await getBuilderOptions(options, mockBuilderContext);
200+
201+
expect(mockedTargetFromTargetString).not.toHaveBeenCalled();
202+
expect(mockBuilderContext.getTargetOptions).not.toHaveBeenCalled();
203+
expect(result).toEqual({
204+
tsConfig: '/test/tsconfig.json',
205+
});
206+
});
207+
208+
it('should handle browser target without configuration', async () => {
209+
const mockTarget = { project: 'test-project', target: 'build' };
210+
mockedTargetFromTargetString.mockReturnValue(mockTarget);
211+
212+
const options: PresetOptions = {
213+
configType: 'DEVELOPMENT',
214+
configDir: '/test/config',
215+
presets: {
216+
apply: vi.fn(),
217+
} as any,
218+
angularBrowserTarget: 'test-project:build',
219+
};
220+
221+
const result = await getBuilderOptions(options, mockBuilderContext);
222+
223+
expect(mockedLogger.info).toHaveBeenCalledWith(
224+
'=> Using angular browser target options from "test-project:build"'
225+
);
226+
});
227+
228+
it('should handle browser target with configuration', async () => {
229+
const mockTarget = { project: 'test-project', target: 'build', configuration: 'production' };
230+
mockedTargetFromTargetString.mockReturnValue(mockTarget);
231+
232+
const options: PresetOptions = {
233+
configType: 'DEVELOPMENT',
234+
configDir: '/test/config',
235+
presets: {
236+
apply: vi.fn(),
237+
} as any,
238+
angularBrowserTarget: 'test-project:build:production',
239+
};
240+
241+
const result = await getBuilderOptions(options, mockBuilderContext);
242+
243+
expect(mockedLogger.info).toHaveBeenCalledWith(
244+
'=> Using angular browser target options from "test-project:build:production"'
245+
);
246+
});
247+
248+
it('should handle empty angularBuilderOptions', async () => {
249+
const mockTarget = { project: 'test-project', target: 'build' };
250+
mockedTargetFromTargetString.mockReturnValue(mockTarget);
251+
252+
const browserTargetOptions = { a: 1, b: 2 };
253+
vi.mocked(mockBuilderContext.getTargetOptions).mockResolvedValue(browserTargetOptions);
254+
255+
const options: PresetOptions = {
256+
configType: 'DEVELOPMENT',
257+
configDir: '/test/config',
258+
presets: {
259+
apply: vi.fn(),
260+
} as any,
261+
angularBrowserTarget: 'test-project:build',
262+
angularBuilderOptions: {},
263+
};
264+
265+
const result = await getBuilderOptions(options, mockBuilderContext);
266+
267+
expect(result).toEqual({
268+
a: 1,
269+
b: 2,
270+
tsConfig: '/test/tsconfig.json',
271+
});
272+
});
273+
274+
it('should handle undefined angularBuilderOptions', async () => {
275+
const mockTarget = { project: 'test-project', target: 'build' };
276+
mockedTargetFromTargetString.mockReturnValue(mockTarget);
277+
278+
const browserTargetOptions = { a: 1, b: 2 };
279+
vi.mocked(mockBuilderContext.getTargetOptions).mockResolvedValue(browserTargetOptions);
280+
281+
const options: PresetOptions = {
282+
configType: 'DEVELOPMENT',
283+
configDir: '/test/config',
284+
presets: {
285+
apply: vi.fn(),
286+
} as any,
287+
angularBrowserTarget: 'test-project:build',
288+
};
289+
290+
const result = await getBuilderOptions(options, mockBuilderContext);
291+
292+
expect(result).toEqual({
293+
a: 1,
294+
b: 2,
295+
tsConfig: '/test/tsconfig.json',
296+
});
297+
});
298+
});
299+
});

‎code/frameworks/angular/src/server/framework-preset-angular-cli.ts

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,38 @@ function getBuilderContext(options: PresetOptions): BuilderContext {
8080
);
8181
}
8282

83+
/**
84+
* Deep merge function that properly handles nested objects. Preserves arrays and objects from
85+
* source when they exist in target
86+
*
87+
* @internal - exported for testing purposes
88+
*/
89+
export function deepMerge(target: JsonObject, source: JsonObject): JsonObject {
90+
const result = { ...target };
91+
92+
for (const key in source) {
93+
if (source[key] !== undefined && source[key] !== null) {
94+
if (
95+
typeof source[key] === 'object' &&
96+
!Array.isArray(source[key]) &&
97+
typeof target[key] === 'object' &&
98+
!Array.isArray(target[key]) &&
99+
target[key] !== null
100+
) {
101+
// Deep merge nested objects
102+
result[key] = deepMerge(target[key] as JsonObject, source[key] as JsonObject);
103+
} else {
104+
// Override with source value
105+
result[key] = source[key];
106+
}
107+
}
108+
}
109+
110+
return result;
111+
}
112+
83113
/** Get builder options Merge target options from browser target and from storybook options */
84-
async function getBuilderOptions(options: PresetOptions, builderContext: BuilderContext) {
114+
export async function getBuilderOptions(options: PresetOptions, builderContext: BuilderContext) {
85115
/** Get Browser Target options */
86116
let browserTargetOptions: JsonObject = {};
87117
if (options.angularBrowserTarget) {
@@ -95,15 +125,19 @@ async function getBuilderOptions(options: PresetOptions, builderContext: Builder
95125
browserTargetOptions = await builderContext.getTargetOptions(browserTarget);
96126
}
97127

98-
/** Merge target options from browser target options and from storybook options */
99-
const builderOptions = {
100-
...browserTargetOptions,
101-
...options.angularBuilderOptions,
102-
tsConfig:
103-
options.tsConfig ??
104-
(await findUp('tsconfig.json', { cwd: options.configDir, stopAt: getProjectRoot() })) ??
105-
browserTargetOptions.tsConfig,
106-
};
128+
/**
129+
* Merge target options from browser target options and from storybook options Use deep merge to
130+
* preserve nested properties like stylePreprocessorOptions.includePaths when they exist in
131+
* browserTarget but not in storybook options
132+
*/
133+
const builderOptions = deepMerge(browserTargetOptions, options.angularBuilderOptions || {});
134+
135+
// Handle tsConfig separately to maintain existing logic
136+
builderOptions.tsConfig =
137+
options.tsConfig ??
138+
(await findUp('tsconfig.json', { cwd: options.configDir, stopAt: getProjectRoot() })) ??
139+
browserTargetOptions.tsConfig;
140+
107141
logger.info(`=> Using angular project with "tsConfig:${builderOptions.tsConfig}"`);
108142

109143
return builderOptions;

0 commit comments

Comments
 (0)