From 97973059ec56a573629f7a367757773a3cfabe17 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Tue, 5 Mar 2024 13:25:04 +0000 Subject: [PATCH] refactor(@angular-devkit/build-angular): remove Sass legacy implementation This commit removes the legacy Sass implementation previously used with Webpack. BREAKING CHANGE: The support for the legacy Sass build pipeline, previously accessible via `NG_BUILD_LEGACY_SASS` when utilizing webpack-based builders, has been removed. --- .../src/tools/sass/sass-service-legacy.ts | 251 ------------------ .../src/tools/sass/worker-legacy.ts | 69 ----- .../src/tools/webpack/configs/styles.ts | 80 ++---- .../src/utils/environment-options.ts | 18 -- .../e2e/tests/build/styles/scss-legacy.ts | 66 ----- 5 files changed, 23 insertions(+), 461 deletions(-) delete mode 100644 packages/angular_devkit/build_angular/src/tools/sass/sass-service-legacy.ts delete mode 100644 packages/angular_devkit/build_angular/src/tools/sass/worker-legacy.ts delete mode 100644 tests/legacy-cli/e2e/tests/build/styles/scss-legacy.ts diff --git a/packages/angular_devkit/build_angular/src/tools/sass/sass-service-legacy.ts b/packages/angular_devkit/build_angular/src/tools/sass/sass-service-legacy.ts deleted file mode 100644 index 02a7ce0f2fff..000000000000 --- a/packages/angular_devkit/build_angular/src/tools/sass/sass-service-legacy.ts +++ /dev/null @@ -1,251 +0,0 @@ -/** - * @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 { join } from 'path'; -import { - LegacyAsyncImporter as AsyncImporter, - LegacyResult as CompileResult, - LegacyException as Exception, - LegacyImporterResult as ImporterResult, - LegacyImporterThis as ImporterThis, - LegacyOptions as Options, - LegacySyncImporter as SyncImporter, -} from 'sass'; -import { MessageChannel, Worker } from 'worker_threads'; -import { maxWorkers } from '../../utils/environment-options'; - -/** - * The maximum number of Workers that will be created to execute render requests. - */ -const MAX_RENDER_WORKERS = maxWorkers; - -/** - * The callback type for the `dart-sass` asynchronous render function. - */ -type RenderCallback = (error?: Exception, result?: CompileResult) => void; - -/** - * An object containing the contextual information for a specific render request. - */ -interface RenderRequest { - id: number; - workerIndex: number; - callback: RenderCallback; - importers?: (SyncImporter | AsyncImporter)[]; -} - -/** - * A response from the Sass render Worker containing the result of the operation. - */ -interface RenderResponseMessage { - id: number; - error?: Exception; - result?: CompileResult; -} - -/** - * A Sass renderer implementation that provides an interface that can be used by Webpack's - * `sass-loader`. The implementation uses a Worker thread to perform the Sass rendering - * with the `dart-sass` package. The `dart-sass` synchronous render function is used within - * the worker which can be up to two times faster than the asynchronous variant. - */ -export class SassLegacyWorkerImplementation { - private readonly workers: Worker[] = []; - private readonly availableWorkers: number[] = []; - private readonly requests = new Map(); - private readonly workerPath = join(__dirname, './worker-legacy.js'); - private idCounter = 1; - private nextWorkerIndex = 0; - - /** - * Provides information about the Sass implementation. - * This mimics enough of the `dart-sass` value to be used with the `sass-loader`. - */ - get info(): string { - return 'dart-sass\tworker'; - } - - /** - * The synchronous render function is not used by the `sass-loader`. - */ - renderSync(): never { - throw new Error('Sass renderSync is not supported.'); - } - - /** - * Asynchronously request a Sass stylesheet to be renderered. - * - * @param options The `dart-sass` options to use when rendering the stylesheet. - * @param callback The function to execute when the rendering is complete. - */ - render(options: Options<'async'>, callback: RenderCallback): void { - // The `functions`, `logger` and `importer` options are JavaScript functions that cannot be transferred. - // If any additional function options are added in the future, they must be excluded as well. - const { functions, importer, logger, ...serializableOptions } = options; - - // The CLI's configuration does not use or expose the ability to defined custom Sass functions - if (functions && Object.keys(functions).length > 0) { - throw new Error('Sass custom functions are not supported.'); - } - - let workerIndex = this.availableWorkers.pop(); - if (workerIndex === undefined) { - if (this.workers.length < MAX_RENDER_WORKERS) { - workerIndex = this.workers.length; - this.workers.push(this.createWorker()); - } else { - workerIndex = this.nextWorkerIndex++; - if (this.nextWorkerIndex >= this.workers.length) { - this.nextWorkerIndex = 0; - } - } - } - - const request = this.createRequest(workerIndex, callback, importer); - this.requests.set(request.id, request); - - this.workers[workerIndex].postMessage({ - id: request.id, - hasImporter: !!importer, - options: serializableOptions, - }); - } - - /** - * Shutdown the Sass render worker. - * Executing this method will stop any pending render requests. - */ - close(): void { - for (const worker of this.workers) { - try { - void worker.terminate(); - } catch {} - } - this.requests.clear(); - } - - private createWorker(): Worker { - const { port1: mainImporterPort, port2: workerImporterPort } = new MessageChannel(); - const importerSignal = new Int32Array(new SharedArrayBuffer(4)); - - const worker = new Worker(this.workerPath, { - workerData: { workerImporterPort, importerSignal }, - transferList: [workerImporterPort], - }); - - worker.on('message', (response: RenderResponseMessage) => { - const request = this.requests.get(response.id); - if (!request) { - return; - } - - this.requests.delete(response.id); - this.availableWorkers.push(request.workerIndex); - - if (response.result) { - // The results are expected to be Node.js `Buffer` objects but will each be transferred as - // a Uint8Array that does not have the expected `toString` behavior of a `Buffer`. - const { css, map, stats } = response.result; - const result: CompileResult = { - // This `Buffer.from` override will use the memory directly and avoid making a copy - css: Buffer.from(css.buffer, css.byteOffset, css.byteLength), - stats, - }; - if (map) { - // This `Buffer.from` override will use the memory directly and avoid making a copy - result.map = Buffer.from(map.buffer, map.byteOffset, map.byteLength); - } - request.callback(undefined, result); - } else { - request.callback(response.error); - } - }); - - mainImporterPort.on( - 'message', - ({ - id, - url, - prev, - fromImport, - }: { - id: number; - url: string; - prev: string; - fromImport: boolean; - }) => { - const request = this.requests.get(id); - if (!request?.importers) { - mainImporterPort.postMessage(null); - Atomics.store(importerSignal, 0, 1); - Atomics.notify(importerSignal, 0); - - return; - } - - this.processImporters(request.importers, url, prev, fromImport) - .then((result) => { - mainImporterPort.postMessage(result); - }) - .catch((error) => { - mainImporterPort.postMessage(error); - }) - .finally(() => { - Atomics.store(importerSignal, 0, 1); - Atomics.notify(importerSignal, 0); - }); - }, - ); - - mainImporterPort.unref(); - - return worker; - } - - private async processImporters( - importers: Iterable, - url: string, - prev: string, - fromImport: boolean, - ): Promise { - let result = null; - for (const importer of importers) { - result = await new Promise((resolve) => { - // Importers can be both sync and async - const innerResult = (importer as AsyncImporter).call( - { fromImport } as ImporterThis, - url, - prev, - resolve, - ); - if (innerResult !== undefined) { - resolve(innerResult); - } - }); - - if (result) { - break; - } - } - - return result; - } - - private createRequest( - workerIndex: number, - callback: RenderCallback, - importer: SyncImporter | AsyncImporter | (SyncImporter | AsyncImporter)[] | undefined, - ): RenderRequest { - return { - id: this.idCounter++, - workerIndex, - callback, - importers: !importer || Array.isArray(importer) ? importer : [importer], - }; - } -} diff --git a/packages/angular_devkit/build_angular/src/tools/sass/worker-legacy.ts b/packages/angular_devkit/build_angular/src/tools/sass/worker-legacy.ts deleted file mode 100644 index bcd978b3258b..000000000000 --- a/packages/angular_devkit/build_angular/src/tools/sass/worker-legacy.ts +++ /dev/null @@ -1,69 +0,0 @@ -/** - * @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 { ImporterResult, LegacyOptions as Options, renderSync } from 'sass'; -import { MessagePort, parentPort, receiveMessageOnPort, workerData } from 'worker_threads'; - -/** - * A request to render a Sass stylesheet using the supplied options. - */ -interface RenderRequestMessage { - /** - * The unique request identifier that links the render action with a callback and optional - * importer on the main thread. - */ - id: number; - /** - * The Sass options to provide to the `dart-sass` render function. - */ - options: Options<'sync'>; - /** - * Indicates the request has a custom importer function on the main thread. - */ - hasImporter: boolean; -} - -if (!parentPort || !workerData) { - throw new Error('Sass worker must be executed as a Worker.'); -} - -// The importer variables are used to proxy import requests to the main thread -const { workerImporterPort, importerSignal } = workerData as { - workerImporterPort: MessagePort; - importerSignal: Int32Array; -}; - -parentPort.on('message', ({ id, hasImporter, options }: RenderRequestMessage) => { - try { - if (hasImporter) { - // When a custom importer function is present, the importer request must be proxied - // back to the main thread where it can be executed. - // This process must be synchronous from the perspective of dart-sass. The `Atomics` - // functions combined with the shared memory `importSignal` and the Node.js - // `receiveMessageOnPort` function are used to ensure synchronous behavior. - options.importer = function (url, prev) { - Atomics.store(importerSignal, 0, 0); - const { fromImport } = this; - workerImporterPort.postMessage({ id, url, prev, fromImport }); - Atomics.wait(importerSignal, 0, 0); - - return receiveMessageOnPort(workerImporterPort)?.message as ImporterResult; - }; - } - - // The synchronous Sass render function can be up to two times faster than the async variant - const result = renderSync(options); - - parentPort?.postMessage({ id, result }); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } catch (error: any) { - // Needed because V8 will only serialize the message and stack properties of an Error instance. - const { formatted, file, line, column, message, stack } = error; - parentPort?.postMessage({ id, error: { formatted, file, line, column, message, stack } }); - } -}); diff --git a/packages/angular_devkit/build_angular/src/tools/webpack/configs/styles.ts b/packages/angular_devkit/build_angular/src/tools/webpack/configs/styles.ts index 889d8ba75c91..da1de3e56e2c 100644 --- a/packages/angular_devkit/build_angular/src/tools/webpack/configs/styles.ts +++ b/packages/angular_devkit/build_angular/src/tools/webpack/configs/styles.ts @@ -12,10 +12,8 @@ import { fileURLToPath, pathToFileURL } from 'node:url'; import type { FileImporter } from 'sass'; import type { Configuration, LoaderContext, RuleSetUseItem } from 'webpack'; import { WebpackConfigOptions } from '../../../utils/build-options'; -import { useLegacySass } from '../../../utils/environment-options'; import { findTailwindConfigurationFile } from '../../../utils/tailwind'; import { SassWorkerImplementation } from '../../sass/sass-service'; -import { SassLegacyWorkerImplementation } from '../../sass/sass-service-legacy'; import { AnyComponentStyleBudgetChecker, PostcssCliResources, @@ -63,9 +61,7 @@ export async function getStylesConfig(wco: WebpackConfigOptions): Promise { - return implementation instanceof SassWorkerImplementation - ? { - sourceMap: true, - api: 'modern', - implementation, - // Webpack importer is only implemented in the legacy API and we have our own custom Webpack importer. - // See: https://github.com/webpack-contrib/sass-loader/blob/997f3eb41d86dd00d5fa49c395a1aeb41573108c/src/utils.js#L642-L651 - webpackImporter: false, - sassOptions: (loaderContext: LoaderContext<{}>) => ({ - importers: [getSassResolutionImporter(loaderContext, root, preserveSymlinks)], - loadPaths: includePaths, - // Use expanded as otherwise sass will remove comments that are needed for autoprefixer - // Ex: /* autoprefixer grid: autoplace */ - // See: https://github.com/webpack-contrib/sass-loader/blob/45ad0be17264ceada5f0b4fb87e9357abe85c4ff/src/getSassOptions.js#L68-L70 - style: 'expanded', - // Silences compiler warnings from 3rd party stylesheets - quietDeps: !verbose, - verbose, - syntax: indentedSyntax ? 'indented' : 'scss', - sourceMapIncludeSources: true, - }), - } - : { - sourceMap: true, - api: 'legacy', - implementation, - sassOptions: { - importer: (url: string, from: string) => { - if (url.charAt(0) === '~') { - throw new Error( - `'${from}' imports '${url}' with a tilde. Usage of '~' in imports is no longer supported.`, - ); - } - - return null; - }, - // Prevent use of `fibers` package as it no longer works in newer Node.js versions - fiber: false, - indentedSyntax, - // bootstrap-sass requires a minimum precision of 8 - precision: 8, - includePaths, - // Use expanded as otherwise sass will remove comments that are needed for autoprefixer - // Ex: /* autoprefixer grid: autoplace */ - // See: https://github.com/webpack-contrib/sass-loader/blob/45ad0be17264ceada5f0b4fb87e9357abe85c4ff/src/getSassOptions.js#L68-L70 - outputStyle: 'expanded', - // Silences compiler warnings from 3rd party stylesheets - quietDeps: !verbose, - verbose, - }, - }; + return { + sourceMap: true, + api: 'modern', + implementation, + // Webpack importer is only implemented in the legacy API and we have our own custom Webpack importer. + // See: https://github.com/webpack-contrib/sass-loader/blob/997f3eb41d86dd00d5fa49c395a1aeb41573108c/src/utils.js#L642-L651 + webpackImporter: false, + sassOptions: (loaderContext: LoaderContext<{}>) => ({ + importers: [getSassResolutionImporter(loaderContext, root, preserveSymlinks)], + loadPaths: includePaths, + // Use expanded as otherwise sass will remove comments that are needed for autoprefixer + // Ex: /* autoprefixer grid: autoplace */ + // See: https://github.com/webpack-contrib/sass-loader/blob/45ad0be17264ceada5f0b4fb87e9357abe85c4ff/src/getSassOptions.js#L68-L70 + style: 'expanded', + // Silences compiler warnings from 3rd party stylesheets + quietDeps: !verbose, + verbose, + syntax: indentedSyntax ? 'indented' : 'scss', + sourceMapIncludeSources: true, + }), + }; } function getSassResolutionImporter( diff --git a/packages/angular_devkit/build_angular/src/utils/environment-options.ts b/packages/angular_devkit/build_angular/src/utils/environment-options.ts index ec2b162cacda..c01c7eddc174 100644 --- a/packages/angular_devkit/build_angular/src/utils/environment-options.ts +++ b/packages/angular_devkit/build_angular/src/utils/environment-options.ts @@ -6,8 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ -import { colors } from './color'; - function isDisabled(variable: string): boolean { return variable === '0' || variable.toLowerCase() === 'false'; } @@ -81,22 +79,6 @@ export const maxWorkers = isPresent(maxWorkersVariable) ? +maxWorkersVariable : const parallelTsVariable = process.env['NG_BUILD_PARALLEL_TS']; export const useParallelTs = !isPresent(parallelTsVariable) || !isDisabled(parallelTsVariable); -const legacySassVariable = process.env['NG_BUILD_LEGACY_SASS']; -export const useLegacySass: boolean = (() => { - if (!isPresent(legacySassVariable)) { - return false; - } - - // eslint-disable-next-line no-console - console.warn( - colors.yellow( - `Warning: 'NG_BUILD_LEGACY_SASS' environment variable support will be removed in version 16.`, - ), - ); - - return isEnabled(legacySassVariable); -})(); - const debugPerfVariable = process.env['NG_BUILD_DEBUG_PERF']; export const debugPerformance = isPresent(debugPerfVariable) && isEnabled(debugPerfVariable); diff --git a/tests/legacy-cli/e2e/tests/build/styles/scss-legacy.ts b/tests/legacy-cli/e2e/tests/build/styles/scss-legacy.ts deleted file mode 100644 index 6c185e3dbca4..000000000000 --- a/tests/legacy-cli/e2e/tests/build/styles/scss-legacy.ts +++ /dev/null @@ -1,66 +0,0 @@ -import { - writeMultipleFiles, - deleteFile, - expectFileToMatch, - replaceInFile, -} from '../../../utils/fs'; -import { expectToFail } from '../../../utils/utils'; -import { execWithEnv } from '../../../utils/process'; -import { updateJsonFile } from '../../../utils/project'; -import assert from 'assert'; - -export default async function () { - await writeMultipleFiles({ - 'src/styles.scss': ` - @import './imported-styles.scss'; - body { background-color: blue; } - `, - 'src/imported-styles.scss': 'p { background-color: red; }', - 'src/app/app.component.scss': ` - .outer { - .inner { - background: #fff; - } - } - `, - }); - - await updateJsonFile('angular.json', (workspaceJson) => { - const appArchitect = workspaceJson.projects['test-project'].architect; - appArchitect.build.options.styles = [{ input: 'src/styles.scss' }]; - }); - - await deleteFile('src/app/app.component.css'); - await replaceInFile('src/app/app.component.ts', './app.component.css', './app.component.scss'); - - const { stderr } = await execWithEnv( - 'ng', - ['build', '--source-map', '--configuration=development'], - { - ...process.env, - NG_BUILD_LEGACY_SASS: '1', - }, - ); - - assert.match( - stderr, - /Warning: 'NG_BUILD_LEGACY_SASS'/, - `Expected stderr to contain 'NG_BUILD_LEGACY_SASS' usage warning`, - ); - - await expectFileToMatch( - 'dist/test-project/browser/styles.css', - /body\s*{\s*background-color: blue;\s*}/, - ); - await expectFileToMatch( - 'dist/test-project/browser/styles.css', - /p\s*{\s*background-color: red;\s*}/, - ); - await expectToFail(() => - expectFileToMatch('dist/test-project/browser/styles.css', '"mappings":""'), - ); - await expectFileToMatch( - 'dist/test-project/browser/main.js', - /.outer.*.inner.*background:\s*#[fF]+/, - ); -}