From a25c88fdbf48f55feacb70496bbd397af31cef7b Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Tue, 27 Sep 2022 07:30:04 +0000 Subject: [PATCH] feat(@angular-devkit/build-angular): switch to use Sass modern API Sass modern API provides faster compilations times when used in an async manner. |Application compilation duration | Sass API and Compiler| |-- | --| |60852ms | dart-sass legacy sync API| |52666ms | dart-sass modern API| Note: https://github.com/johannesjo/super-productivity was used for benchmarking. Prior art: http://docs/document/d/1CvEceWMpBoEBd8SfvksGMdVHxaZMH93b0EGS3XbR3_Q?resourcekey=0-vFm-xMspT65FZLIyX7xWFQ BREAKING CHANGE: - Deprecated support for tilde import has been removed. Please update the imports by removing the `~`. Before ```scss @import "~font-awesome/scss/font-awesome"; ``` After ```scss @import "font-awesome/scss/font-awesome"; ``` - By default the CLI will use Sass modern API, While not recommended, users can still opt to use legacy API by setting `NG_BUILD_LEGACY_SASS=1`. --- .../src/builders/browser/specs/styles_spec.ts | 26 +- .../src/sass/sass-service-legacy.ts | 251 ++++++++++++++++++ .../build_angular/src/sass/sass-service.ts | 177 ++++++------ .../build_angular/src/sass/worker-legacy.ts | 69 +++++ .../build_angular/src/sass/worker.ts | 77 ++++-- .../src/utils/environment-options.ts | 18 ++ .../src/webpack/configs/styles.ts | 158 ++++++----- .../e2e/tests/build/styles/scss-legacy.ts | 55 ++++ .../legacy-cli/e2e/tests/build/styles/scss.ts | 46 ++-- 9 files changed, 662 insertions(+), 215 deletions(-) create mode 100644 packages/angular_devkit/build_angular/src/sass/sass-service-legacy.ts create mode 100644 packages/angular_devkit/build_angular/src/sass/worker-legacy.ts create mode 100644 tests/legacy-cli/e2e/tests/build/styles/scss-legacy.ts diff --git a/packages/angular_devkit/build_angular/src/builders/browser/specs/styles_spec.ts b/packages/angular_devkit/build_angular/src/builders/browser/specs/styles_spec.ts index 7c0015d02e99..2b2d8eb9e9d1 100644 --- a/packages/angular_devkit/build_angular/src/builders/browser/specs/styles_spec.ts +++ b/packages/angular_devkit/build_angular/src/builders/browser/specs/styles_spec.ts @@ -195,11 +195,9 @@ describe('Browser Builder styles', () => { it(`supports material imports in ${ext} files`, async () => { host.writeMultipleFiles({ [`src/styles.${ext}`]: ` - @import "~@angular/material/prebuilt-themes/indigo-pink.css"; @import "@angular/material/prebuilt-themes/indigo-pink.css"; `, [`src/app/app.component.${ext}`]: ` - @import "~@angular/material/prebuilt-themes/indigo-pink.css"; @import "@angular/material/prebuilt-themes/indigo-pink.css"; `, }); @@ -265,19 +263,14 @@ describe('Browser Builder styles', () => { }); }); - /** - * font-awesome mock to avoid having an extra dependency. - */ - function mockFontAwesomePackage(host: TestProjectHost): void { + it(`supports font-awesome imports`, async () => { + // font-awesome mock to avoid having an extra dependency. host.writeMultipleFiles({ 'node_modules/font-awesome/scss/font-awesome.scss': ` - * { color: red } + * { color: red } `, }); - } - it(`supports font-awesome imports`, async () => { - mockFontAwesomePackage(host); host.writeMultipleFiles({ 'src/styles.scss': ` @import "font-awesome/scss/font-awesome"; @@ -288,19 +281,6 @@ describe('Browser Builder styles', () => { await browserBuild(architect, host, target, overrides); }); - it(`supports font-awesome imports (tilde)`, async () => { - mockFontAwesomePackage(host); - host.writeMultipleFiles({ - 'src/styles.scss': ` - $fa-font-path: "~font-awesome/fonts"; - @import "~font-awesome/scss/font-awesome"; - `, - }); - - const overrides = { styles: [`src/styles.scss`] }; - await browserBuild(architect, host, target, overrides); - }); - it(`uses autoprefixer`, async () => { host.writeMultipleFiles({ 'src/styles.css': tags.stripIndents` diff --git a/packages/angular_devkit/build_angular/src/sass/sass-service-legacy.ts b/packages/angular_devkit/build_angular/src/sass/sass-service-legacy.ts new file mode 100644 index 000000000000..e1f64abad2d7 --- /dev/null +++ b/packages/angular_devkit/build_angular/src/sass/sass-service-legacy.ts @@ -0,0 +1,251 @@ +/** + * @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/sass/sass-service.ts b/packages/angular_devkit/build_angular/src/sass/sass-service.ts index 4a40334412d8..ae3dd99335b8 100644 --- a/packages/angular_devkit/build_angular/src/sass/sass-service.ts +++ b/packages/angular_devkit/build_angular/src/sass/sass-service.ts @@ -6,15 +6,16 @@ * 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, + CompileResult, + Exception, + FileImporter, + Importer, + StringOptionsWithImporter, + StringOptionsWithoutImporter, } from 'sass'; +import { fileURLToPath, pathToFileURL } from 'url'; import { MessageChannel, Worker } from 'worker_threads'; import { maxWorkers } from '../utils/environment-options'; @@ -28,6 +29,8 @@ const MAX_RENDER_WORKERS = maxWorkers; */ type RenderCallback = (error?: Exception, result?: CompileResult) => void; +type FileImporterOptions = Parameters[1]; + /** * An object containing the contextual information for a specific render request. */ @@ -35,16 +38,25 @@ interface RenderRequest { id: number; workerIndex: number; callback: RenderCallback; - importers?: (SyncImporter | AsyncImporter)[]; + importers?: Importers[]; } +/** + * All available importer types. + */ +type Importers = + | Importer<'sync'> + | Importer<'async'> + | FileImporter<'sync'> + | FileImporter<'async'>; + /** * A response from the Sass render Worker containing the result of the operation. */ interface RenderResponseMessage { id: number; error?: Exception; - result?: CompileResult; + result?: Omit & { loadedUrls: string[] }; } /** @@ -57,6 +69,7 @@ export class SassWorkerImplementation { private readonly workers: Worker[] = []; private readonly availableWorkers: number[] = []; private readonly requests = new Map(); + private readonly workerPath = join(__dirname, './worker.js'); private idCounter = 1; private nextWorkerIndex = 0; @@ -71,46 +84,77 @@ export class SassWorkerImplementation { /** * The synchronous render function is not used by the `sass-loader`. */ - renderSync(): never { - throw new Error('Sass renderSync is not supported.'); + compileString(): never { + throw new Error('Sass compileString is not supported.'); } /** * Asynchronously request a Sass stylesheet to be renderered. * + * @param source The contents to compile. * @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 { + compileStringAsync( + source: string, + options: StringOptionsWithImporter<'async'> | StringOptionsWithoutImporter<'async'>, + ): Promise { // 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; + const { functions, importers, url, 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; + return new Promise((resolve, reject) => { + 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); + const callback: RenderCallback = (error, result) => { + if (error) { + const url = error?.span.url as string | undefined; + if (url) { + error.span.url = pathToFileURL(url); + } - this.workers[workerIndex].postMessage({ - id: request.id, - hasImporter: !!importer, - options: serializableOptions, + reject(error); + + return; + } + + if (!result) { + reject(new Error('No result.')); + + return; + } + + resolve(result); + }; + + const request = this.createRequest(workerIndex, callback, importers); + this.requests.set(request.id, request); + + this.workers[workerIndex].postMessage({ + id: request.id, + source, + hasImporter: !!importers?.length, + options: { + ...serializableOptions, + // URL is not serializable so to convert to string here and back to URL in the worker. + url: url ? fileURLToPath(url) : undefined, + }, + }); }); } @@ -131,8 +175,7 @@ export class SassWorkerImplementation { const { port1: mainImporterPort, port2: workerImporterPort } = new MessageChannel(); const importerSignal = new Int32Array(new SharedArrayBuffer(4)); - const workerPath = require.resolve('./worker'); - const worker = new Worker(workerPath, { + const worker = new Worker(this.workerPath, { workerData: { workerImporterPort, importerSignal }, transferList: [workerImporterPort], }); @@ -147,19 +190,11 @@ export class SassWorkerImplementation { 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); + request.callback(undefined, { + ...response.result, + // URL is not serializable so in the worker we convert to string and here back to URL. + loadedUrls: response.result.loadedUrls.map((p) => pathToFileURL(p)), + }); } else { request.callback(response.error); } @@ -167,17 +202,7 @@ export class SassWorkerImplementation { mainImporterPort.on( 'message', - ({ - id, - url, - prev, - fromImport, - }: { - id: number; - url: string; - prev: string; - fromImport: boolean; - }) => { + ({ id, url, options }: { id: number; url: string; options: FileImporterOptions }) => { const request = this.requests.get(id); if (!request?.importers) { mainImporterPort.postMessage(null); @@ -187,7 +212,7 @@ export class SassWorkerImplementation { return; } - this.processImporters(request.importers, url, prev, fromImport) + this.processImporters(request.importers, url, options) .then((result) => { mainImporterPort.postMessage(result); }) @@ -207,44 +232,40 @@ export class SassWorkerImplementation { } private async processImporters( - importers: Iterable, + importers: Iterable, url: string, - prev: string, - fromImport: boolean, - ): Promise { - let result = null; + options: FileImporterOptions, + ): Promise { 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 (this.isImporter(importer)) { + // Importer + throw new Error('Only File Importers are supported.'); + } + // File importer (Can be sync or aync). + const result = await importer.findFileUrl(url, options); if (result) { - break; + return fileURLToPath(result); } } - return result; + return null; } private createRequest( workerIndex: number, callback: RenderCallback, - importer: SyncImporter | AsyncImporter | (SyncImporter | AsyncImporter)[] | undefined, + importers: Importers[] | undefined, ): RenderRequest { return { id: this.idCounter++, workerIndex, callback, - importers: !importer || Array.isArray(importer) ? importer : [importer], + importers, }; } + + private isImporter(value: Importers): value is Importer { + return 'canonicalize' in value && 'load' in value; + } } diff --git a/packages/angular_devkit/build_angular/src/sass/worker-legacy.ts b/packages/angular_devkit/build_angular/src/sass/worker-legacy.ts new file mode 100644 index 000000000000..bcd978b3258b --- /dev/null +++ b/packages/angular_devkit/build_angular/src/sass/worker-legacy.ts @@ -0,0 +1,69 @@ +/** + * @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/sass/worker.ts b/packages/angular_devkit/build_angular/src/sass/worker.ts index bcd978b3258b..65d168e009d0 100644 --- a/packages/angular_devkit/build_angular/src/sass/worker.ts +++ b/packages/angular_devkit/build_angular/src/sass/worker.ts @@ -6,7 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ -import { ImporterResult, LegacyOptions as Options, renderSync } from 'sass'; +import { Exception, StringOptionsWithImporter, compileString } from 'sass'; +import { fileURLToPath, pathToFileURL } from 'url'; import { MessagePort, parentPort, receiveMessageOnPort, workerData } from 'worker_threads'; /** @@ -19,9 +20,13 @@ interface RenderRequestMessage { */ id: number; /** - * The Sass options to provide to the `dart-sass` render function. + * The contents to compile. */ - options: Options<'sync'>; + source: string; + /** + * The Sass options to provide to the `dart-sass` compile function. + */ + options: Omit, 'url'> & { url?: string }; /** * Indicates the request has a custom importer function on the main thread. */ @@ -38,7 +43,11 @@ const { workerImporterPort, importerSignal } = workerData as { importerSignal: Int32Array; }; -parentPort.on('message', ({ id, hasImporter, options }: RenderRequestMessage) => { +parentPort.on('message', ({ id, hasImporter, source, options }: RenderRequestMessage) => { + if (!parentPort) { + throw new Error('"parentPort" is not defined. Sass worker must be executed as a Worker.'); + } + try { if (hasImporter) { // When a custom importer function is present, the importer request must be proxied @@ -46,24 +55,58 @@ parentPort.on('message', ({ id, hasImporter, options }: RenderRequestMessage) => // 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); + options.importers = [ + { + findFileUrl: (url, options) => { + Atomics.store(importerSignal, 0, 0); + workerImporterPort.postMessage({ id, url, options }); + Atomics.wait(importerSignal, 0, 0); + + const result = receiveMessageOnPort(workerImporterPort)?.message as string | null; - return receiveMessageOnPort(workerImporterPort)?.message as ImporterResult; - }; + return result ? pathToFileURL(result) : null; + }, + }, + ]; } // The synchronous Sass render function can be up to two times faster than the async variant - const result = renderSync(options); + const result = compileString(source, { + ...options, + // URL is not serializable so to convert to string in the parent and back to URL here. + url: options.url ? pathToFileURL(options.url) : undefined, + }); - parentPort?.postMessage({ id, result }); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } catch (error: any) { + parentPort.postMessage({ + id, + result: { + ...result, + // URL is not serializable so to convert to string here and back to URL in the parent. + loadedUrls: result.loadedUrls.map((p) => fileURLToPath(p)), + }, + }); + } catch (error) { // 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 } }); + if (error instanceof Exception) { + const { span, message, stack, sassMessage, sassStack } = error; + parentPort.postMessage({ + id, + error: { + span: { + ...span, + url: span.url ? fileURLToPath(span.url) : undefined, + }, + message, + stack, + sassMessage, + sassStack, + }, + }); + } else if (error instanceof Error) { + const { message, stack } = error; + parentPort.postMessage({ id, error: { message, stack } }); + } else { + parentPort.postMessage({ id, error: { message: 'An unknown error has occurred.' } }); + } } }); 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 d896cef0d87b..6cf761a4bf5d 100644 --- a/packages/angular_devkit/build_angular/src/utils/environment-options.ts +++ b/packages/angular_devkit/build_angular/src/utils/environment-options.ts @@ -6,6 +6,8 @@ * 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'; } @@ -75,3 +77,19 @@ export const allowMinify = debugOptimize.minify; */ const maxWorkersVariable = process.env['NG_BUILD_MAX_WORKERS']; export const maxWorkers = isPresent(maxWorkersVariable) ? +maxWorkersVariable : 4; + +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); +})(); diff --git a/packages/angular_devkit/build_angular/src/webpack/configs/styles.ts b/packages/angular_devkit/build_angular/src/webpack/configs/styles.ts index dd1a318055dc..85f420f356ed 100644 --- a/packages/angular_devkit/build_angular/src/webpack/configs/styles.ts +++ b/packages/angular_devkit/build_angular/src/webpack/configs/styles.ts @@ -9,11 +9,12 @@ import * as fs from 'fs'; import MiniCssExtractPlugin from 'mini-css-extract-plugin'; import * as path from 'path'; -import type { Configuration, RuleSetUseItem } from 'webpack'; +import { Configuration, RuleSetUseItem } from 'webpack'; import { StyleElement } from '../../builders/browser/schema'; import { SassWorkerImplementation } from '../../sass/sass-service'; +import { SassLegacyWorkerImplementation } from '../../sass/sass-service-legacy'; import { WebpackConfigOptions } from '../../utils/build-options'; -import { addWarning } from '../../utils/webpack-diagnostics'; +import { useLegacySass } from '../../utils/environment-options'; import { AnyComponentStyleBudgetChecker, PostcssCliResources, @@ -77,7 +78,7 @@ export function resolveGlobalStyles( // eslint-disable-next-line max-lines-per-function export function getStylesConfig(wco: WebpackConfigOptions): Configuration { - const { root, buildOptions } = wco; + const { root, projectRoot, buildOptions } = wco; const extraPlugins: Configuration['plugins'] = []; extraPlugins.push(new AnyComponentStyleBudgetChecker(buildOptions.budgets)); @@ -102,22 +103,15 @@ export function getStylesConfig(wco: WebpackConfigOptions): Configuration { extraPlugins.push(new RemoveHashPlugin({ chunkNames: noInjectNames, hashFormat })); } - const sassImplementation = new SassWorkerImplementation(); - const sassTildeUsageMessage = new Set(); + const sassImplementation = useLegacySass + ? new SassLegacyWorkerImplementation() + : new SassWorkerImplementation(); extraPlugins.push({ apply(compiler) { compiler.hooks.shutdown.tap('sass-worker', () => { sassImplementation.close(); }); - - compiler.hooks.afterCompile.tap('sass-worker', (compilation) => { - for (const message of sassTildeUsageMessage) { - addWarning(compilation, message); - } - - sassTildeUsageMessage.clear(); - }); }, }); @@ -161,7 +155,6 @@ export function getStylesConfig(wco: WebpackConfigOptions): Configuration { : undefined, plugins: [ postcssImports({ - resolve: (url: string) => (url.startsWith('~') ? url.slice(1) : url), load: (filename: string) => { return new Promise((resolve, reject) => { loader.fs.readFile(filename, (err: Error, data: Buffer) => { @@ -272,33 +265,14 @@ export function getStylesConfig(wco: WebpackConfigOptions): Configuration { }, { loader: require.resolve('sass-loader'), - options: { - implementation: sassImplementation, - sourceMap: true, - sassOptions: { - importer: (url: string, from: string) => { - if (url.charAt(0) === '~') { - sassTildeUsageMessage.add( - `'${from}' imports '${url}' with a tilde. Usage of '~' in imports is deprecated.`, - ); - } - - return null; - }, - // Prevent use of `fibers` package as it no longer works in newer Node.js versions - fiber: false, - // 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: !buildOptions.verbose, - verbose: buildOptions.verbose, - }, - }, + options: getSassLoaderOptions( + root, + projectRoot, + sassImplementation, + includePaths, + false, + !buildOptions.verbose, + ), }, ], }, @@ -313,34 +287,14 @@ export function getStylesConfig(wco: WebpackConfigOptions): Configuration { }, { loader: require.resolve('sass-loader'), - options: { - implementation: sassImplementation, - sourceMap: true, - sassOptions: { - importer: (url: string, from: string) => { - if (url.charAt(0) === '~') { - sassTildeUsageMessage.add( - `'${from}' imports '${url}' with a tilde. Usage of '~' in imports is deprecated.`, - ); - } - - return null; - }, - // Prevent use of `fibers` package as it no longer works in newer Node.js versions - fiber: false, - indentedSyntax: true, - // 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: !buildOptions.verbose, - verbose: buildOptions.verbose, - }, - }, + options: getSassLoaderOptions( + root, + projectRoot, + sassImplementation, + includePaths, + true, + !buildOptions.verbose, + ), }, ], }, @@ -419,3 +373,69 @@ function getTailwindConfigPath({ projectRoot, root }: WebpackConfigOptions): str return undefined; } + +function getSassLoaderOptions( + root: string, + projectRoot: string, + implementation: SassWorkerImplementation | SassLegacyWorkerImplementation, + includePaths: string[], + indentedSyntax: boolean, + verbose: boolean, +): Record { + return implementation instanceof SassWorkerImplementation + ? { + sourceMap: true, + api: 'modern', + implementation, + // Webpack importer is only implemented in the legacy API. + // See: https://github.com/webpack-contrib/sass-loader/blob/997f3eb41d86dd00d5fa49c395a1aeb41573108c/src/utils.js#L642-L651 + webpackImporter: false, + sassOptions: { + loadPaths: [ + ...includePaths, + // Needed to resolve node packages and retain the same behaviour of with the legacy API as sass-loader resolves + // scss also from the cwd and project root. + // See: https://github.com/webpack-contrib/sass-loader/blob/997f3eb41d86dd00d5fa49c395a1aeb41573108c/src/utils.js#L307 + projectRoot, + path.join(root, 'node_modules'), + ], + // 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', + }, + } + : { + 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, + }, + }; +} diff --git a/tests/legacy-cli/e2e/tests/build/styles/scss-legacy.ts b/tests/legacy-cli/e2e/tests/build/styles/scss-legacy.ts new file mode 100644 index 000000000000..c872d3170d53 --- /dev/null +++ b/tests/legacy-cli/e2e/tests/build/styles/scss-legacy.ts @@ -0,0 +1,55 @@ +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/styles.css', /body\s*{\s*background-color: blue;\s*}/); + await expectFileToMatch('dist/test-project/styles.css', /p\s*{\s*background-color: red;\s*}/); + await expectToFail(() => expectFileToMatch('dist/test-project/styles.css', '"mappings":""')); + await expectFileToMatch('dist/test-project/main.js', /.outer.*.inner.*background:\s*#[fF]+/); +} diff --git a/tests/legacy-cli/e2e/tests/build/styles/scss.ts b/tests/legacy-cli/e2e/tests/build/styles/scss.ts index 1fc269f38830..6c68c1fc8240 100644 --- a/tests/legacy-cli/e2e/tests/build/styles/scss.ts +++ b/tests/legacy-cli/e2e/tests/build/styles/scss.ts @@ -8,10 +8,8 @@ import { expectToFail } from '../../../utils/utils'; import { ng } from '../../../utils/process'; import { updateJsonFile } from '../../../utils/project'; -export default function () { - // TODO(architect): Delete this test. It is now in devkit/build-angular. - - return writeMultipleFiles({ +export default async function () { + await writeMultipleFiles({ 'src/styles.scss': ` @import './imported-styles.scss'; body { background-color: blue; } @@ -24,28 +22,20 @@ export default function () { } } `, - }) - .then(() => deleteFile('src/app/app.component.css')) - .then(() => - updateJsonFile('angular.json', (workspaceJson) => { - const appArchitect = workspaceJson.projects['test-project'].architect; - appArchitect.build.options.styles = [{ input: 'src/styles.scss' }]; - }), - ) - .then(() => - replaceInFile('src/app/app.component.ts', './app.component.css', './app.component.scss'), - ) - .then(() => ng('build', '--source-map', '--configuration=development')) - .then(() => - expectFileToMatch('dist/test-project/styles.css', /body\s*{\s*background-color: blue;\s*}/), - ) - .then(() => - expectFileToMatch('dist/test-project/styles.css', /p\s*{\s*background-color: red;\s*}/), - ) - .then(() => - expectToFail(() => expectFileToMatch('dist/test-project/styles.css', '"mappings":""')), - ) - .then(() => - expectFileToMatch('dist/test-project/main.js', /.outer.*.inner.*background:\s*#[fF]+/), - ); + }); + + 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'); + + await ng('build', '--source-map', '--configuration=development'); + + await expectFileToMatch('dist/test-project/styles.css', /body\s*{\s*background-color: blue;\s*}/); + await expectFileToMatch('dist/test-project/styles.css', /p\s*{\s*background-color: red;\s*}/); + await expectToFail(() => expectFileToMatch('dist/test-project/styles.css', '"mappings":""')); + await expectFileToMatch('dist/test-project/main.js', /.outer.*.inner.*background:\s*#[fF]+/); }