From 425f04f0158b9f083154a1d6def536207f32780b Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Wed, 12 Jul 2023 10:24:35 -0400 Subject: [PATCH] perf(@angular-devkit/build-angular): inject Sass import/use directive importer information when resolving To correctly resolve a package based import reference in a Sass file with pnpm or Yarn PnP, the importer file path must be known. Unfortunately, the Sass compiler does not provided the importer file to import plugins. Previously to workaround this issue, all previously resolved stylesheets were tried as the importer path. This allowed the stylesheets to be resolved but it also could cause a potentially large increase in build time due to the amount of previous stylesheets that would need to be tried. To avoid the performance impact and to also provide more accurate information regarding the importer file, a lexer is now used to extract import information for a stylesheet and inject the importer file path into the specifier. This information is then extracted from the import specifier during the Sass resolution process and allows the underlying package resolution access to a viable location to resolve the package for all package managers. This information is currently limited to specifiers referencing the `@angular` and `@material` package scopes but a comprehensive pre-resolution process may be added in the future. --- .../esbuild/stylesheets/sass-language.ts | 52 ++-- .../build_angular/src/tools/sass/lexer.ts | 267 ++++++++++++++++++ .../src/tools/sass/rebasing-importer.ts | 226 +++++---------- .../src/tools/sass/sass-service.ts | 6 + 4 files changed, 355 insertions(+), 196 deletions(-) create mode 100644 packages/angular_devkit/build_angular/src/tools/sass/lexer.ts diff --git a/packages/angular_devkit/build_angular/src/tools/esbuild/stylesheets/sass-language.ts b/packages/angular_devkit/build_angular/src/tools/esbuild/stylesheets/sass-language.ts index eb4a0c9d532c..a6ee5a760e00 100644 --- a/packages/angular_devkit/build_angular/src/tools/esbuild/stylesheets/sass-language.ts +++ b/packages/angular_devkit/build_angular/src/tools/esbuild/stylesheets/sass-language.ts @@ -33,17 +33,21 @@ export const SassStylesheetLanguage = Object.freeze({ fileFilter: /\.s[ac]ss$/, process(data, file, format, options, build) { const syntax = format === 'sass' ? 'indented' : 'scss'; - const resolveUrl = async (url: string, previousResolvedModules?: Set) => { + const resolveUrl = async (url: string, options: FileImporterWithRequestContextOptions) => { let result = await build.resolve(url, { kind: 'import-rule', - // This should ideally be the directory of the importer file from Sass - // but that is not currently available from the Sass importer API. - resolveDir: build.initialOptions.absWorkingDir, + // Use the provided resolve directory from the custom Sass service if available + resolveDir: options.resolveDir ?? build.initialOptions.absWorkingDir, }); - // Workaround to support Yarn PnP without access to the importer file from Sass - if (!result.path && previousResolvedModules?.size) { - for (const previous of previousResolvedModules) { + // If a resolve directory is provided, no additional speculative resolutions are required + if (options.resolveDir) { + return result; + } + + // Workaround to support Yarn PnP and pnpm without access to the importer file from Sass + if (!result.path && options.previousResolvedModules?.size) { + for (const previous of options.previousResolvedModules) { result = await build.resolve(url, { kind: 'import-rule', resolveDir: previous, @@ -66,7 +70,10 @@ async function compileString( filePath: string, syntax: Syntax, options: StylesheetPluginOptions, - resolveUrl: (url: string, previousResolvedModules?: Set) => Promise, + resolveUrl: ( + url: string, + options: FileImporterWithRequestContextOptions, + ) => Promise, ): Promise { // Lazily load Sass when a Sass file is found if (sassWorkerPool === undefined) { @@ -88,9 +95,9 @@ async function compileString( { findFileUrl: async ( url, - { previousResolvedModules }: FileImporterWithRequestContextOptions, + options: FileImporterWithRequestContextOptions, ): Promise => { - let result = await resolveUrl(url); + const result = await resolveUrl(url, options); if (result.path) { return pathToFileURL(result.path); } @@ -101,30 +108,7 @@ async function compileString( const [nameOrScope, nameOrFirstPath, ...pathPart] = parts; const packageName = hasScope ? `${nameOrScope}/${nameOrFirstPath}` : nameOrScope; - let packageResult = await resolveUrl(packageName + '/package.json'); - - if (packageResult.path) { - return pathToFileURL( - join( - dirname(packageResult.path), - !hasScope && nameOrFirstPath ? nameOrFirstPath : '', - ...pathPart, - ), - ); - } - - // Check with Yarn PnP workaround using previous resolved modules. - // This is done last to avoid a performance penalty for common cases. - - result = await resolveUrl(url, previousResolvedModules); - if (result.path) { - return pathToFileURL(result.path); - } - - packageResult = await resolveUrl( - packageName + '/package.json', - previousResolvedModules, - ); + const packageResult = await resolveUrl(packageName + '/package.json', options); if (packageResult.path) { return pathToFileURL( diff --git a/packages/angular_devkit/build_angular/src/tools/sass/lexer.ts b/packages/angular_devkit/build_angular/src/tools/sass/lexer.ts new file mode 100644 index 000000000000..9d35edb33a41 --- /dev/null +++ b/packages/angular_devkit/build_angular/src/tools/sass/lexer.ts @@ -0,0 +1,267 @@ +/** + * @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 + */ + +// TODO: Combine everything into a single pass lexer + +/** + * Determines if a unicode code point is a CSS whitespace character. + * @param code The unicode code point to test. + * @returns true, if the code point is CSS whitespace; false, otherwise. + */ +function isWhitespace(code: number): boolean { + // Based on https://www.w3.org/TR/css-syntax-3/#whitespace + switch (code) { + case 0x0009: // tab + case 0x0020: // space + case 0x000a: // line feed + case 0x000c: // form feed + case 0x000d: // carriage return + return true; + default: + return false; + } +} + +/** + * Scans a CSS or Sass file and locates all valid url function values as defined by the + * syntax specification. + * @param contents A string containing a CSS or Sass file to scan. + * @returns An iterable that yields each CSS url function value found. + */ +export function* findUrls( + contents: string, +): Iterable<{ start: number; end: number; value: string }> { + let pos = 0; + let width = 1; + let current = -1; + const next = () => { + pos += width; + current = contents.codePointAt(pos) ?? -1; + width = current > 0xffff ? 2 : 1; + + return current; + }; + + // Based on https://www.w3.org/TR/css-syntax-3/#consume-ident-like-token + while ((pos = contents.indexOf('url(', pos)) !== -1) { + // Set to position of the ( + pos += 3; + width = 1; + + // Consume all leading whitespace + while (isWhitespace(next())) { + /* empty */ + } + + // Initialize URL state + const url = { start: pos, end: -1, value: '' }; + let complete = false; + + // If " or ', then consume the value as a string + if (current === 0x0022 || current === 0x0027) { + const ending = current; + // Based on https://www.w3.org/TR/css-syntax-3/#consume-string-token + while (!complete) { + switch (next()) { + case -1: // EOF + return; + case 0x000a: // line feed + case 0x000c: // form feed + case 0x000d: // carriage return + // Invalid + complete = true; + break; + case 0x005c: // \ -- character escape + // If not EOF or newline, add the character after the escape + switch (next()) { + case -1: + return; + case 0x000a: // line feed + case 0x000c: // form feed + case 0x000d: // carriage return + // Skip when inside a string + break; + default: + // TODO: Handle hex escape codes + url.value += String.fromCodePoint(current); + break; + } + break; + case ending: + // Full string position should include the quotes for replacement + url.end = pos + 1; + complete = true; + yield url; + break; + default: + url.value += String.fromCodePoint(current); + break; + } + } + + next(); + continue; + } + + // Based on https://www.w3.org/TR/css-syntax-3/#consume-url-token + while (!complete) { + switch (current) { + case -1: // EOF + return; + case 0x0022: // " + case 0x0027: // ' + case 0x0028: // ( + // Invalid + complete = true; + break; + case 0x0029: // ) + // URL is valid and complete + url.end = pos; + complete = true; + break; + case 0x005c: // \ -- character escape + // If not EOF or newline, add the character after the escape + switch (next()) { + case -1: // EOF + return; + case 0x000a: // line feed + case 0x000c: // form feed + case 0x000d: // carriage return + // Invalid + complete = true; + break; + default: + // TODO: Handle hex escape codes + url.value += String.fromCodePoint(current); + break; + } + break; + default: + if (isWhitespace(current)) { + while (isWhitespace(next())) { + /* empty */ + } + // Unescaped whitespace is only valid before the closing ) + if (current === 0x0029) { + // URL is valid + url.end = pos; + } + complete = true; + } else { + // Add the character to the url value + url.value += String.fromCodePoint(current); + } + break; + } + next(); + } + + // An end position indicates a URL was found + if (url.end !== -1) { + yield url; + } + } +} + +/** + * Scans a CSS or Sass file and locates all valid import/use directive values as defined by the + * syntax specification. + * @param contents A string containing a CSS or Sass file to scan. + * @returns An iterable that yields each CSS directive value found. + */ +export function* findImports( + contents: string, +): Iterable<{ start: number; end: number; specifier: string }> { + yield* find(contents, '@import '); + yield* find(contents, '@use '); +} + +/** + * Scans a CSS or Sass file and locates all valid function/directive values as defined by the + * syntax specification. + * @param contents A string containing a CSS or Sass file to scan. + * @param prefix The prefix to start a valid segment. + * @returns An iterable that yields each CSS url function value found. + */ +function* find( + contents: string, + prefix: string, +): Iterable<{ start: number; end: number; specifier: string }> { + let pos = 0; + let width = 1; + let current = -1; + const next = () => { + pos += width; + current = contents.codePointAt(pos) ?? -1; + width = current > 0xffff ? 2 : 1; + + return current; + }; + + // Based on https://www.w3.org/TR/css-syntax-3/#consume-ident-like-token + while ((pos = contents.indexOf(prefix, pos)) !== -1) { + // Set to position of the last character in prefix + pos += prefix.length - 1; + width = 1; + + // Consume all leading whitespace + while (isWhitespace(next())) { + /* empty */ + } + + // Initialize URL state + const url = { start: pos, end: -1, specifier: '' }; + let complete = false; + + // If " or ', then consume the value as a string + if (current === 0x0022 || current === 0x0027) { + const ending = current; + // Based on https://www.w3.org/TR/css-syntax-3/#consume-string-token + while (!complete) { + switch (next()) { + case -1: // EOF + return; + case 0x000a: // line feed + case 0x000c: // form feed + case 0x000d: // carriage return + // Invalid + complete = true; + break; + case 0x005c: // \ -- character escape + // If not EOF or newline, add the character after the escape + switch (next()) { + case -1: + return; + case 0x000a: // line feed + case 0x000c: // form feed + case 0x000d: // carriage return + // Skip when inside a string + break; + default: + // TODO: Handle hex escape codes + url.specifier += String.fromCodePoint(current); + break; + } + break; + case ending: + // Full string position should include the quotes for replacement + url.end = pos + 1; + complete = true; + yield url; + break; + default: + url.specifier += String.fromCodePoint(current); + break; + } + } + + next(); + continue; + } + } +} diff --git a/packages/angular_devkit/build_angular/src/tools/sass/rebasing-importer.ts b/packages/angular_devkit/build_angular/src/tools/sass/rebasing-importer.ts index 7e9fc7960755..736f3c28d6a9 100644 --- a/packages/angular_devkit/build_angular/src/tools/sass/rebasing-importer.ts +++ b/packages/angular_devkit/build_angular/src/tools/sass/rebasing-importer.ts @@ -11,7 +11,8 @@ import MagicString from 'magic-string'; import { readFileSync, readdirSync } from 'node:fs'; import { basename, dirname, extname, join, relative } from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; -import type { FileImporter, Importer, ImporterResult, Syntax } from 'sass'; +import type { Importer, ImporterResult, Syntax } from 'sass'; +import { findImports, findUrls } from './lexer'; /** * A preprocessed cache entry for the files and directories within a previously searched @@ -22,6 +23,38 @@ export interface DirectoryEntry { directories: Set; } +/** + * A prefix that is added to import and use directive specifiers that should be resolved + * as modules and that will contain added resolve directory information. + * + * This functionality is used to workaround the Sass limitation that it does not provide the + * importer file to custom resolution plugins. + */ +const MODULE_RESOLUTION_PREFIX = '__NG_PACKAGE__'; + +function packModuleSpecifier(specifier: string, resolveDir: string): string { + const packed = MODULE_RESOLUTION_PREFIX + ';' + resolveDir + ';' + specifier; + + // Normalize path separators and escape characters + // https://developer.mozilla.org/en-US/docs/Web/CSS/url#syntax + const normalizedPacked = packed.replace(/\\/g, '/').replace(/[()\s'"]/g, '\\$&'); + + return normalizedPacked; +} + +function unpackModuleSpecifier(specifier: string): { specifier: string; resolveDir?: string } { + if (!specifier.startsWith(`${MODULE_RESOLUTION_PREFIX};`)) { + return { specifier }; + } + + const values = specifier.split(';', 3); + + return { + specifier: values[2], + resolveDir: values[1], + }; +} + /** * A Sass Importer base class that provides the load logic to rebase all `url()` functions * within a stylesheet. The rebasing will ensure that the URLs in the output of the Sass compiler @@ -74,6 +107,27 @@ abstract class UrlRebasingImporter implements Importer<'sync'> { updatedContents.update(start, end, rebasedUrl); } + // Add resolution directory information to module specifiers to facilitate resolution + for (const { start, end, specifier } of findImports(contents)) { + // Currently only provide directory information for known/common packages: + // * `@material/` + // * `@angular/` + // + // Comprehensive pre-resolution support may be added in the future. This is complicated by CSS/Sass not + // requiring a `./` or `../` prefix to signify relative paths. A bare specifier could be either relative + // or a module specifier. To differentiate, a relative resolution would need to be attempted first. + if (!specifier.startsWith('@angular/') && !specifier.startsWith('@material/')) { + continue; + } + + updatedContents ??= new MagicString(contents); + updatedContents.update( + start, + end, + `"${packModuleSpecifier(specifier, stylesheetDirectory)}"`, + ); + } + if (updatedContents) { contents = updatedContents.toString(); if (this.rebaseSourceMaps) { @@ -108,164 +162,6 @@ abstract class UrlRebasingImporter implements Importer<'sync'> { } } -/** - * Determines if a unicode code point is a CSS whitespace character. - * @param code The unicode code point to test. - * @returns true, if the code point is CSS whitespace; false, otherwise. - */ -function isWhitespace(code: number): boolean { - // Based on https://www.w3.org/TR/css-syntax-3/#whitespace - switch (code) { - case 0x0009: // tab - case 0x0020: // space - case 0x000a: // line feed - case 0x000c: // form feed - case 0x000d: // carriage return - return true; - default: - return false; - } -} - -/** - * Scans a CSS or Sass file and locates all valid url function values as defined by the CSS - * syntax specification. - * @param contents A string containing a CSS or Sass file to scan. - * @returns An iterable that yields each CSS url function value found. - */ -function* findUrls(contents: string): Iterable<{ start: number; end: number; value: string }> { - let pos = 0; - let width = 1; - let current = -1; - const next = () => { - pos += width; - current = contents.codePointAt(pos) ?? -1; - width = current > 0xffff ? 2 : 1; - - return current; - }; - - // Based on https://www.w3.org/TR/css-syntax-3/#consume-ident-like-token - while ((pos = contents.indexOf('url(', pos)) !== -1) { - // Set to position of the ( - pos += 3; - width = 1; - - // Consume all leading whitespace - while (isWhitespace(next())) { - /* empty */ - } - - // Initialize URL state - const url = { start: pos, end: -1, value: '' }; - let complete = false; - - // If " or ', then consume the value as a string - if (current === 0x0022 || current === 0x0027) { - const ending = current; - // Based on https://www.w3.org/TR/css-syntax-3/#consume-string-token - while (!complete) { - switch (next()) { - case -1: // EOF - return; - case 0x000a: // line feed - case 0x000c: // form feed - case 0x000d: // carriage return - // Invalid - complete = true; - break; - case 0x005c: // \ -- character escape - // If not EOF or newline, add the character after the escape - switch (next()) { - case -1: - return; - case 0x000a: // line feed - case 0x000c: // form feed - case 0x000d: // carriage return - // Skip when inside a string - break; - default: - // TODO: Handle hex escape codes - url.value += String.fromCodePoint(current); - break; - } - break; - case ending: - // Full string position should include the quotes for replacement - url.end = pos + 1; - complete = true; - yield url; - break; - default: - url.value += String.fromCodePoint(current); - break; - } - } - - next(); - continue; - } - - // Based on https://www.w3.org/TR/css-syntax-3/#consume-url-token - while (!complete) { - switch (current) { - case -1: // EOF - return; - case 0x0022: // " - case 0x0027: // ' - case 0x0028: // ( - // Invalid - complete = true; - break; - case 0x0029: // ) - // URL is valid and complete - url.end = pos; - complete = true; - break; - case 0x005c: // \ -- character escape - // If not EOF or newline, add the character after the escape - switch (next()) { - case -1: // EOF - return; - case 0x000a: // line feed - case 0x000c: // form feed - case 0x000d: // carriage return - // Invalid - complete = true; - break; - default: - // TODO: Handle hex escape codes - url.value += String.fromCodePoint(current); - break; - } - break; - default: - if (isWhitespace(current)) { - while (isWhitespace(next())) { - /* empty */ - } - // Unescaped whitespace is only valid before the closing ) - if (current === 0x0029) { - // URL is valid - url.end = pos; - } - complete = true; - } else { - // Add the character to the url value - url.value += String.fromCodePoint(current); - } - break; - } - next(); - } - - // An end position indicates a URL was found - if (url.end !== -1) { - yield url; - } - } -} - /** * Provides the Sass importer logic to resolve relative stylesheet imports via both import and use rules * and also rebase any `url()` function usage within those stylesheets. The rebasing will ensure that @@ -445,7 +341,10 @@ export class ModuleUrlRebasingImporter extends RelativeUrlRebasingImporter { entryDirectory: string, directoryCache: Map, rebaseSourceMaps: Map | undefined, - private finder: FileImporter<'sync'>['findFileUrl'], + private finder: ( + specifier: string, + options: { fromImport: boolean; resolveDir?: string }, + ) => URL | null, ) { super(entryDirectory, directoryCache, rebaseSourceMaps); } @@ -455,9 +354,12 @@ export class ModuleUrlRebasingImporter extends RelativeUrlRebasingImporter { return super.canonicalize(url, options); } - const result = this.finder(url, options); + const { specifier, resolveDir } = unpackModuleSpecifier(url); - return result ? super.canonicalize(result.href, options) : null; + let result = this.finder(specifier, { ...options, resolveDir }); + result &&= super.canonicalize(result.href, options); + + return result; } } diff --git a/packages/angular_devkit/build_angular/src/tools/sass/sass-service.ts b/packages/angular_devkit/build_angular/src/tools/sass/sass-service.ts index a4e23211f783..eb525325aae5 100644 --- a/packages/angular_devkit/build_angular/src/tools/sass/sass-service.ts +++ b/packages/angular_devkit/build_angular/src/tools/sass/sass-service.ts @@ -41,6 +41,12 @@ export interface FileImporterWithRequestContextOptions extends FileImporterOptio * Workaround until https://github.com/sass/sass/issues/3247 is addressed. */ previousResolvedModules?: Set; + + /** + * The base directory to use when resolving the request. + * This value is only set if using the rebasing importers. + */ + resolveDir?: string; } /**