Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): handle conditional exports in `sc…
Browse files Browse the repository at this point in the history
…ripts` and `styles` option

With this change scripts and styles options better support Yarn PNP resolution.

Closes #23568
  • Loading branch information
alan-agius4 committed Sep 30, 2022
1 parent f64c3bc commit b6df9c1
Show file tree
Hide file tree
Showing 9 changed files with 163 additions and 103 deletions.
Expand Up @@ -21,7 +21,7 @@ import { generateEntryPoints } from '../../utils/package-chunk-sort';
import { augmentAppWithServiceWorker } from '../../utils/service-worker';
import { getSupportedBrowsers } from '../../utils/supported-browsers';
import { getIndexInputFile, getIndexOutputFile } from '../../utils/webpack-browser-config';
import { resolveGlobalStyles } from '../../webpack/configs';
import { normalizeGlobalStyles } from '../../webpack/utils/helpers';
import { createCompilerPlugin } from './compiler-plugin';
import { bundle, logMessages } from './esbuild';
import { logExperimentalWarnings } from './experimental-warnings';
Expand Down Expand Up @@ -347,13 +347,8 @@ async function bundleGlobalStylesheets(
const warnings: Message[] = [];

// resolveGlobalStyles is temporarily reused from the Webpack builder code
const { entryPoints: stylesheetEntrypoints, noInjectNames } = resolveGlobalStyles(
const { entryPoints: stylesheetEntrypoints, noInjectNames } = normalizeGlobalStyles(
options.styles || [],
workspaceRoot,
// preserveSymlinks is always true here to allow the bundler to handle the option
true,
// skipResolution to leverage the bundler's more comprehensive resolution
true,
);

for (const [name, files] of Object.entries(stylesheetEntrypoints)) {
Expand Down
Expand Up @@ -145,12 +145,4 @@ describe('Browser Builder scripts array', () => {
expect(joinedLogs).toMatch(/renamed-lazy-script.+\d+ bytes/);
expect(joinedLogs).not.toContain('Lazy Chunks');
});

it(`should error when a script doesn't exist`, async () => {
await expectAsync(
browserBuild(architect, host, target, {
scripts: ['./invalid.js'],
}),
).toBeRejectedWithError(`Script file ./invalid.js does not exist.`);
});
});
Expand Up @@ -93,18 +93,19 @@ describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
);
});

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

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

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

Expand Down
Expand Up @@ -121,7 +121,10 @@ describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {

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

harness.expectFile('dist/styles.css').toNotExist();
Expand Down
Expand Up @@ -147,10 +147,7 @@ export async function getCommonConfig(wco: WebpackConfigOptions): Promise<Config

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

Expand All @@ -160,7 +157,7 @@ export async function getCommonConfig(wco: WebpackConfigOptions): Promise<Config
sourceMap: scriptsSourceMap,
scripts: paths,
filename: `${path.basename(bundleName)}${hash}.js`,
basePath: projectRoot,
basePath: root,
}),
);
}
Expand Down
76 changes: 17 additions & 59 deletions packages/angular_devkit/build_angular/src/webpack/configs/styles.ts
Expand Up @@ -24,60 +24,14 @@ import {
SuppressExtractedTextChunksWebpackPlugin,
} from '../plugins';
import { CssOptimizerPlugin } from '../plugins/css-optimizer-plugin';
import { StylesWebpackPlugin } from '../plugins/styles-webpack-plugin';
import {
assetNameTemplateFactory,
getOutputHashFormat,
normalizeExtraEntryPoints,
normalizeGlobalStyles,
} from '../utils/helpers';

export function resolveGlobalStyles(
styleEntrypoints: StyleElement[],
root: string,
preserveSymlinks: boolean,
skipResolution = false,
): { entryPoints: Record<string, string[]>; noInjectNames: string[]; paths: string[] } {
const entryPoints: Record<string, string[]> = {};
const noInjectNames: string[] = [];
const paths: string[] = [];

if (styleEntrypoints.length === 0) {
return { entryPoints, noInjectNames, paths };
}

for (const style of normalizeExtraEntryPoints(styleEntrypoints, 'styles')) {
let stylesheetPath = style.input;
if (!skipResolution) {
stylesheetPath = path.resolve(root, stylesheetPath);
if (!fs.existsSync(stylesheetPath)) {
try {
stylesheetPath = require.resolve(style.input, { paths: [root] });
} catch {}
}
}

if (!preserveSymlinks) {
stylesheetPath = fs.realpathSync(stylesheetPath);
}

// Add style entry points.
if (entryPoints[style.bundleName]) {
entryPoints[style.bundleName].push(stylesheetPath);
} else {
entryPoints[style.bundleName] = [stylesheetPath];
}

// Add non injected styles to the list.
if (!style.inject) {
noInjectNames.push(style.bundleName);
}

// Add global css paths.
paths.push(stylesheetPath);
}

return { entryPoints, noInjectNames, paths };
}

// eslint-disable-next-line max-lines-per-function
export function getStylesConfig(wco: WebpackConfigOptions): Configuration {
const { root, projectRoot, buildOptions } = wco;
Expand All @@ -95,14 +49,20 @@ export function getStylesConfig(wco: WebpackConfigOptions): Configuration {
buildOptions.stylePreprocessorOptions?.includePaths?.map((p) => path.resolve(root, p)) ?? [];

// Process global styles.
const {
entryPoints,
noInjectNames,
paths: globalStylePaths,
} = resolveGlobalStyles(buildOptions.styles, root, !!buildOptions.preserveSymlinks);
if (noInjectNames.length > 0) {
// Add plugin to remove hashes from lazy styles.
extraPlugins.push(new RemoveHashPlugin({ chunkNames: noInjectNames, hashFormat }));
if (buildOptions.styles.length > 0) {
const { entryPoints, noInjectNames } = normalizeGlobalStyles(buildOptions.styles);
extraPlugins.push(
new StylesWebpackPlugin({
root,
entryPoints,
preserveSymlinks: buildOptions.preserveSymlinks,
}),
);

if (noInjectNames.length > 0) {
// Add plugin to remove hashes from lazy styles.
extraPlugins.push(new RemoveHashPlugin({ chunkNames: noInjectNames, hashFormat }));
}
}

const sassImplementation = useLegacySass
Expand Down Expand Up @@ -319,7 +279,6 @@ export function getStylesConfig(wco: WebpackConfigOptions): Configuration {
];

return {
entry: entryPoints,
module: {
rules: styleLanguages.map(({ extensions, use }) => ({
test: new RegExp(`\\.(?:${extensions.join('|')})$`, 'i'),
Expand All @@ -330,8 +289,7 @@ export function getStylesConfig(wco: WebpackConfigOptions): Configuration {
// Global styles are only defined global styles
{
use: globalStyleLoaders,
include: globalStylePaths,
resourceQuery: { not: [/\?ngResource/] },
resourceQuery: /\?ngGlobalStyle/,
},
// Component styles are all styles except defined global styles
{
Expand Down
Expand Up @@ -9,6 +9,8 @@
import { interpolateName } from 'loader-utils';
import * as path from 'path';
import { Chunk, Compilation, Compiler, sources as webpackSources } from 'webpack';
import { assertIsError } from '../../utils/error';
import { addError } from '../../utils/webpack-diagnostics';

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

Expand All @@ -35,6 +37,7 @@ function addDependencies(compilation: Compilation, scripts: string[]): void {
compilation.fileDependencies.add(script);
}
}

export class ScriptsWebpackPlugin {
private _lastBuildTime?: number;
private _cachedOutput?: ScriptOutput;
Expand Down Expand Up @@ -88,21 +91,38 @@ export class ScriptsWebpackPlugin {
compilation.entrypoints.set(this.options.name, entrypoint);
compilation.chunks.add(chunk);

// eslint-disable-next-line @typescript-eslint/no-explicit-any
compilation.assets[filename] = source as any;
compilation.assets[filename] = source;
compilation.hooks.chunkAsset.call(chunk, filename);
}

apply(compiler: Compiler): void {
if (!this.options.scripts || this.options.scripts.length === 0) {
if (this.options.scripts.length === 0) {
return;
}

const scripts = this.options.scripts
.filter((script) => !!script)
.map((script) => path.resolve(this.options.basePath || '', script));
const resolver = compiler.resolverFactory.get('normal', {
preferRelative: true,
useSyncFileSystemCalls: true,
fileSystem: compiler.inputFileSystem,
});

compiler.hooks.thisCompilation.tap(PLUGIN_NAME, (compilation) => {
const scripts: string[] = [];

for (const script of this.options.scripts) {
try {
const resolvedPath = resolver.resolveSync({}, this.options.basePath, script);
if (resolvedPath) {
scripts.push(resolvedPath);
} else {
addError(compilation, `Cannot resolve '${script}'.`);
}
} catch (error) {
assertIsError(error);
addError(compilation, error.message);
}
}

compilation.hooks.additionalAssets.tapPromise(PLUGIN_NAME, async () => {
if (await this.shouldSkip(compilation, scripts)) {
if (this._cachedOutput) {
Expand Down
@@ -0,0 +1,79 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import assert from 'assert';
import { pluginName } from 'mini-css-extract-plugin';
import type { Compilation, Compiler } from 'webpack';
import { assertIsError } from '../../utils/error';
import { addError } from '../../utils/webpack-diagnostics';

export interface StylesWebpackPluginOptions {
preserveSymlinks?: boolean;
root: string;
entryPoints: Record<string, string[]>;
}

/**
* The name of the plugin provided to Webpack when tapping Webpack compiler hooks.
*/
const PLUGIN_NAME = 'styles-webpack-plugin';

export class StylesWebpackPlugin {
private compilation: Compilation | undefined;

constructor(private readonly options: StylesWebpackPluginOptions) {}

apply(compiler: Compiler): void {
const { entryPoints, preserveSymlinks, root } = this.options;
const webpackOptions = compiler.options;
const entry =
typeof webpackOptions.entry === 'function' ? webpackOptions.entry() : webpackOptions.entry;

const resolver = compiler.resolverFactory.get('global-styles', {
conditionNames: ['sass', 'less', 'style'],
mainFields: ['sass', 'less', 'style', 'main', '...'],
extensions: ['.scss', '.sass', '.less', '.css'],
restrictions: [/\.((le|sa|sc|c)ss)$/i],
preferRelative: true,
useSyncFileSystemCalls: true,
symlinks: !preserveSymlinks,
fileSystem: compiler.inputFileSystem,
});

webpackOptions.entry = async () => {
const entrypoints = await entry;

for (const [bundleName, paths] of Object.entries(entryPoints)) {
entrypoints[bundleName] ??= {};
const entryImport = (entrypoints[bundleName].import ??= []);

for (const path of paths) {
try {
const resolvedPath = resolver.resolveSync({}, root, path);
if (resolvedPath) {
entryImport.push(`${resolvedPath}?ngGlobalStyle`);
} else {
assert(this.compilation, 'Compilation cannot be undefined.');
addError(this.compilation, `Cannot resolve '${path}'.`);
}
} catch (error) {
assert(this.compilation, 'Compilation cannot be undefined.');
assertIsError(error);
addError(this.compilation, error.message);
}
}
}

return entrypoints;
};

compiler.hooks.thisCompilation.tap(PLUGIN_NAME, (compilation) => {
this.compilation = compilation;
});
}
}

0 comments on commit b6df9c1

Please sign in to comment.