Skip to content

Commit f7ec116

Browse files
committed
fix(@angular-devkit/build-angular): handle conditional exports in scripts and styles option
With this change scripts and styles options better support Yarn PNP resolution. Closes #23568
1 parent 91a6bd4 commit f7ec116

File tree

9 files changed

+160
-101
lines changed

9 files changed

+160
-101
lines changed

packages/angular_devkit/build_angular/src/builders/browser-esbuild/index.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { generateEntryPoints } from '../../utils/package-chunk-sort';
2121
import { augmentAppWithServiceWorker } from '../../utils/service-worker';
2222
import { getSupportedBrowsers } from '../../utils/supported-browsers';
2323
import { getIndexInputFile, getIndexOutputFile } from '../../utils/webpack-browser-config';
24-
import { resolveGlobalStyles } from '../../webpack/configs';
24+
import { normalizeGlobalStyles } from '../../webpack/utils/helpers';
2525
import { createCompilerPlugin } from './compiler-plugin';
2626
import { bundle, logMessages } from './esbuild';
2727
import { logExperimentalWarnings } from './experimental-warnings';
@@ -347,13 +347,8 @@ async function bundleGlobalStylesheets(
347347
const warnings: Message[] = [];
348348

349349
// resolveGlobalStyles is temporarily reused from the Webpack builder code
350-
const { entryPoints: stylesheetEntrypoints, noInjectNames } = resolveGlobalStyles(
350+
const { entryPoints: stylesheetEntrypoints, noInjectNames } = normalizeGlobalStyles(
351351
options.styles || [],
352-
workspaceRoot,
353-
// preserveSymlinks is always true here to allow the bundler to handle the option
354-
true,
355-
// skipResolution to leverage the bundler's more comprehensive resolution
356-
true,
357352
);
358353

359354
for (const [name, files] of Object.entries(stylesheetEntrypoints)) {

packages/angular_devkit/build_angular/src/builders/browser/specs/scripts-array_spec.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -145,12 +145,4 @@ describe('Browser Builder scripts array', () => {
145145
expect(joinedLogs).toMatch(/renamed-lazy-script.+\d+ bytes/);
146146
expect(joinedLogs).not.toContain('Lazy Chunks');
147147
});
148-
149-
it(`should error when a script doesn't exist`, async () => {
150-
await expectAsync(
151-
browserBuild(architect, host, target, {
152-
scripts: ['./invalid.js'],
153-
}),
154-
).toBeRejectedWithError(`Script file ./invalid.js does not exist.`);
155-
});
156148
});

packages/angular_devkit/build_angular/src/builders/browser/tests/options/scripts_spec.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,18 +93,19 @@ describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
9393
);
9494
});
9595

96-
it('throws an exception if script does not exist', async () => {
96+
it('fails and shows an error if script does not exist', async () => {
9797
harness.useTarget('build', {
9898
...BASE_OPTIONS,
9999
scripts: ['src/test-script-a.js'],
100100
});
101101

102-
const { result, error } = await harness.executeOnce({ outputLogsOnException: false });
102+
const { result, logs } = await harness.executeOnce();
103103

104-
expect(result).toBeUndefined();
105-
expect(error).toEqual(
104+
expect(result?.success).toBeFalse();
105+
expect(logs).toContain(
106106
jasmine.objectContaining({
107-
message: jasmine.stringMatching(`Script file src/test-script-a.js does not exist.`),
107+
level: 'error',
108+
message: jasmine.stringMatching(`Can't resolve 'src/test-script-a.js'`),
108109
}),
109110
);
110111

packages/angular_devkit/build_angular/src/builders/browser/tests/options/styles_spec.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,10 @@ describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
121121

122122
expect(result?.success).toBeFalse();
123123
expect(logs).toContain(
124-
jasmine.objectContaining({ message: jasmine.stringMatching('Module not found:') }),
124+
jasmine.objectContaining({
125+
level: 'error',
126+
message: jasmine.stringMatching(`Can't resolve 'src/test-style-a.css'`),
127+
}),
125128
);
126129

127130
harness.expectFile('dist/styles.css').toNotExist();

packages/angular_devkit/build_angular/src/webpack/configs/common.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -147,10 +147,7 @@ export async function getCommonConfig(wco: WebpackConfigOptions): Promise<Config
147147

148148
// process global scripts
149149
// Add a new asset for each entry.
150-
for (const { bundleName, inject, paths } of globalScriptsByBundleName(
151-
root,
152-
buildOptions.scripts,
153-
)) {
150+
for (const { bundleName, inject, paths } of globalScriptsByBundleName(buildOptions.scripts)) {
154151
// Lazy scripts don't get a hash, otherwise they can't be loaded by name.
155152
const hash = inject ? hashFormat.script : '';
156153

@@ -160,7 +157,7 @@ export async function getCommonConfig(wco: WebpackConfigOptions): Promise<Config
160157
sourceMap: scriptsSourceMap,
161158
scripts: paths,
162159
filename: `${path.basename(bundleName)}${hash}.js`,
163-
basePath: projectRoot,
160+
basePath: root,
164161
}),
165162
);
166163
}

packages/angular_devkit/build_angular/src/webpack/configs/styles.ts

Lines changed: 17 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -22,60 +22,14 @@ import {
2222
SuppressExtractedTextChunksWebpackPlugin,
2323
} from '../plugins';
2424
import { CssOptimizerPlugin } from '../plugins/css-optimizer-plugin';
25+
import { StylesWebpackPlugin } from '../plugins/styles-webpack-plugin';
2526
import {
2627
assetNameTemplateFactory,
2728
getOutputHashFormat,
2829
normalizeExtraEntryPoints,
30+
normalizeGlobalStyles,
2931
} from '../utils/helpers';
3032

31-
export function resolveGlobalStyles(
32-
styleEntrypoints: StyleElement[],
33-
root: string,
34-
preserveSymlinks: boolean,
35-
skipResolution = false,
36-
): { entryPoints: Record<string, string[]>; noInjectNames: string[]; paths: string[] } {
37-
const entryPoints: Record<string, string[]> = {};
38-
const noInjectNames: string[] = [];
39-
const paths: string[] = [];
40-
41-
if (styleEntrypoints.length === 0) {
42-
return { entryPoints, noInjectNames, paths };
43-
}
44-
45-
for (const style of normalizeExtraEntryPoints(styleEntrypoints, 'styles')) {
46-
let stylesheetPath = style.input;
47-
if (!skipResolution) {
48-
stylesheetPath = path.resolve(root, stylesheetPath);
49-
if (!fs.existsSync(stylesheetPath)) {
50-
try {
51-
stylesheetPath = require.resolve(style.input, { paths: [root] });
52-
} catch {}
53-
}
54-
}
55-
56-
if (!preserveSymlinks) {
57-
stylesheetPath = fs.realpathSync(stylesheetPath);
58-
}
59-
60-
// Add style entry points.
61-
if (entryPoints[style.bundleName]) {
62-
entryPoints[style.bundleName].push(stylesheetPath);
63-
} else {
64-
entryPoints[style.bundleName] = [stylesheetPath];
65-
}
66-
67-
// Add non injected styles to the list.
68-
if (!style.inject) {
69-
noInjectNames.push(style.bundleName);
70-
}
71-
72-
// Add global css paths.
73-
paths.push(stylesheetPath);
74-
}
75-
76-
return { entryPoints, noInjectNames, paths };
77-
}
78-
7933
// eslint-disable-next-line max-lines-per-function
8034
export function getStylesConfig(wco: WebpackConfigOptions): Configuration {
8135
const { root, projectRoot, buildOptions } = wco;
@@ -93,14 +47,20 @@ export function getStylesConfig(wco: WebpackConfigOptions): Configuration {
9347
buildOptions.stylePreprocessorOptions?.includePaths?.map((p) => path.resolve(root, p)) ?? [];
9448

9549
// Process global styles.
96-
const {
97-
entryPoints,
98-
noInjectNames,
99-
paths: globalStylePaths,
100-
} = resolveGlobalStyles(buildOptions.styles, root, !!buildOptions.preserveSymlinks);
101-
if (noInjectNames.length > 0) {
102-
// Add plugin to remove hashes from lazy styles.
103-
extraPlugins.push(new RemoveHashPlugin({ chunkNames: noInjectNames, hashFormat }));
50+
if (buildOptions.styles.length > 0) {
51+
const { entryPoints, noInjectNames } = normalizeGlobalStyles(buildOptions.styles);
52+
extraPlugins.push(
53+
new StylesWebpackPlugin({
54+
root,
55+
entryPoints,
56+
preserveSymlinks: buildOptions.preserveSymlinks,
57+
}),
58+
);
59+
60+
if (noInjectNames.length > 0) {
61+
// Add plugin to remove hashes from lazy styles.
62+
extraPlugins.push(new RemoveHashPlugin({ chunkNames: noInjectNames, hashFormat }));
63+
}
10464
}
10565

10666
const sassImplementation = useLegacySass
@@ -317,7 +277,6 @@ export function getStylesConfig(wco: WebpackConfigOptions): Configuration {
317277
];
318278

319279
return {
320-
entry: entryPoints,
321280
module: {
322281
rules: styleLanguages.map(({ extensions, use }) => ({
323282
test: new RegExp(`\\.(?:${extensions.join('|')})$`, 'i'),
@@ -328,8 +287,7 @@ export function getStylesConfig(wco: WebpackConfigOptions): Configuration {
328287
// Global styles are only defined global styles
329288
{
330289
use: globalStyleLoaders,
331-
include: globalStylePaths,
332-
resourceQuery: { not: [/\?ngResource/] },
290+
resourceQuery: /\?ngGlobalStyle/,
333291
},
334292
// Component styles are all styles except defined global styles
335293
{

packages/angular_devkit/build_angular/src/webpack/plugins/scripts-webpack-plugin.ts

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@
88

99
import { interpolateName } from 'loader-utils';
1010
import * as path from 'path';
11+
import { assertIsError } from '../../utils/error';
1112
import { Chunk, Compilation, Compiler, sources as webpackSources } from 'webpack';
13+
import { addError } from '../../utils/webpack-diagnostics';
1214

1315
const Entrypoint = require('webpack/lib/Entrypoint');
1416

@@ -35,6 +37,7 @@ function addDependencies(compilation: Compilation, scripts: string[]): void {
3537
compilation.fileDependencies.add(script);
3638
}
3739
}
40+
3841
export class ScriptsWebpackPlugin {
3942
private _lastBuildTime?: number;
4043
private _cachedOutput?: ScriptOutput;
@@ -94,15 +97,33 @@ export class ScriptsWebpackPlugin {
9497
}
9598

9699
apply(compiler: Compiler): void {
97-
if (!this.options.scripts || this.options.scripts.length === 0) {
100+
if (!this.options.scripts.length) {
98101
return;
99102
}
100103

101-
const scripts = this.options.scripts
102-
.filter((script) => !!script)
103-
.map((script) => path.resolve(this.options.basePath || '', script));
104+
const resolver = compiler.resolverFactory.get('normal', {
105+
preferRelative: true,
106+
useSyncFileSystemCalls: true,
107+
fileSystem: compiler.inputFileSystem,
108+
});
104109

105110
compiler.hooks.thisCompilation.tap(PLUGIN_NAME, (compilation) => {
111+
const scripts: string[] = [];
112+
113+
for (const script of this.options.scripts) {
114+
try {
115+
const resolvedPath = resolver.resolveSync({}, this.options.basePath, script);
116+
if (resolvedPath) {
117+
scripts.push(resolvedPath);
118+
} else {
119+
addError(compilation, `Cannot resolve '${script}'.`);
120+
}
121+
} catch (error) {
122+
assertIsError(error);
123+
addError(compilation, error.message);
124+
}
125+
}
126+
106127
compilation.hooks.additionalAssets.tapPromise(PLUGIN_NAME, async () => {
107128
if (await this.shouldSkip(compilation, scripts)) {
108129
if (this._cachedOutput) {
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import assert from 'assert';
10+
import type { Compilation, Compiler } from 'webpack';
11+
import { assertIsError } from '../../utils/error';
12+
import { addError } from '../../utils/webpack-diagnostics';
13+
14+
export interface StylesWebpackPluginOptions {
15+
preserveSymlinks?: boolean;
16+
root: string;
17+
entryPoints: Record<string, string[]>;
18+
}
19+
20+
/**
21+
* The name of the plugin provided to Webpack when tapping Webpack compiler hooks.
22+
*/
23+
const PLUGIN_NAME = 'styles-webpack-plugin';
24+
25+
export class StylesWebpackPlugin {
26+
private compilation: Compilation | undefined;
27+
28+
constructor(private readonly options: StylesWebpackPluginOptions) {}
29+
30+
apply(compiler: Compiler): void {
31+
const { entryPoints, preserveSymlinks, root } = this.options;
32+
const webpackOptions = compiler.options;
33+
const entry =
34+
typeof webpackOptions.entry === 'function' ? webpackOptions.entry() : webpackOptions.entry;
35+
36+
const resolver = compiler.resolverFactory.get('global-styles', {
37+
conditionNames: ['sass', 'less', 'style'],
38+
mainFields: ['sass', 'less', 'style', 'main', '...'],
39+
extensions: ['.scss', '.sass', '.less', '.css'],
40+
restrictions: [/\.((le|sa|sc|c)ss)$/i],
41+
preferRelative: true,
42+
useSyncFileSystemCalls: true,
43+
symlinks: !preserveSymlinks,
44+
fileSystem: compiler.inputFileSystem,
45+
});
46+
47+
webpackOptions.entry = async () => {
48+
assert(this.compilation, 'Compilation cannot be undefined.');
49+
const entrypoints = await entry;
50+
51+
for (const [bundleName, paths] of Object.entries(entryPoints)) {
52+
entrypoints[bundleName] ??= {};
53+
const entryImport = (entrypoints[bundleName].import ??= []);
54+
55+
for (const path of paths) {
56+
try {
57+
const resolvedPath = resolver.resolveSync({}, root, path);
58+
if (resolvedPath) {
59+
entryImport.push(`${resolvedPath}?ngGlobalStyle`);
60+
} else {
61+
addError(this.compilation, `Cannot resolve '${path}'.`);
62+
}
63+
} catch (error) {
64+
assertIsError(error);
65+
addError(this.compilation, error.message);
66+
}
67+
}
68+
}
69+
70+
return entrypoints;
71+
};
72+
73+
compiler.hooks.thisCompilation.tap(PLUGIN_NAME, (compilation) => {
74+
this.compilation = compilation;
75+
});
76+
}
77+
}

0 commit comments

Comments
 (0)