Skip to content

Commit 0862a38

Browse files
committed
refactor(@angular-devkit/build-angular): remove Sass module resolve workarounds
The recent version of the Sass compiler (`dart-sass@1.68.0`) now provides additional information within an importer that allows more accurate resolution of node modules packages without several existing workarounds. Previously, the Sass files needed to be pre-processed to extract all `@import` and `@use` paths so that information regarding the containing Sass file could be used to fully resolve the paths. The Sass compiler now provides this information directly.
1 parent babe467 commit 0862a38

File tree

5 files changed

+40
-149
lines changed

5 files changed

+40
-149
lines changed

packages/angular_devkit/build_angular/src/tools/esbuild/stylesheets/sass-language.ts

+12-32
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,8 @@
99
import type { OnLoadResult, PartialMessage, ResolveResult } from 'esbuild';
1010
import { dirname, join, relative } from 'node:path';
1111
import { fileURLToPath, pathToFileURL } from 'node:url';
12-
import type { CompileResult, Exception, Syntax } from 'sass';
13-
import type {
14-
FileImporterWithRequestContextOptions,
15-
SassWorkerImplementation,
16-
} from '../../sass/sass-service';
12+
import type { CanonicalizeContext, CompileResult, Exception, Syntax } from 'sass';
13+
import type { SassWorkerImplementation } from '../../sass/sass-service';
1714
import { StylesheetLanguage, StylesheetPluginOptions } from './stylesheet-plugin-factory';
1815

1916
let sassWorkerPool: SassWorkerImplementation | undefined;
@@ -39,30 +36,16 @@ export const SassStylesheetLanguage = Object.freeze<StylesheetLanguage>({
3936
fileFilter: /\.s[ac]ss$/,
4037
process(data, file, format, options, build) {
4138
const syntax = format === 'sass' ? 'indented' : 'scss';
42-
const resolveUrl = async (url: string, options: FileImporterWithRequestContextOptions) => {
43-
let result = await build.resolve(url, {
44-
kind: 'import-rule',
45-
// Use the provided resolve directory from the custom Sass service if available
46-
resolveDir: options.resolveDir ?? build.initialOptions.absWorkingDir,
47-
});
48-
49-
// If a resolve directory is provided, no additional speculative resolutions are required
50-
if (options.resolveDir) {
51-
return result;
39+
const resolveUrl = async (url: string, options: CanonicalizeContext) => {
40+
let resolveDir = build.initialOptions.absWorkingDir;
41+
if (options.containingUrl) {
42+
resolveDir = dirname(fileURLToPath(options.containingUrl));
5243
}
5344

54-
// Workaround to support Yarn PnP and pnpm without access to the importer file from Sass
55-
if (!result.path && options.previousResolvedModules?.size) {
56-
for (const previous of options.previousResolvedModules) {
57-
result = await build.resolve(url, {
58-
kind: 'import-rule',
59-
resolveDir: previous,
60-
});
61-
if (result.path) {
62-
break;
63-
}
64-
}
65-
}
45+
const result = await build.resolve(url, {
46+
kind: 'import-rule',
47+
resolveDir,
48+
});
6649

6750
return result;
6851
};
@@ -103,10 +86,7 @@ async function compileString(
10386
filePath: string,
10487
syntax: Syntax,
10588
options: StylesheetPluginOptions,
106-
resolveUrl: (
107-
url: string,
108-
options: FileImporterWithRequestContextOptions,
109-
) => Promise<ResolveResult>,
89+
resolveUrl: (url: string, options: CanonicalizeContext) => Promise<ResolveResult>,
11090
): Promise<OnLoadResult> {
11191
// Lazily load Sass when a Sass file is found
11292
if (sassWorkerPool === undefined) {
@@ -139,7 +119,7 @@ async function compileString(
139119
quietDeps: true,
140120
importers: [
141121
{
142-
findFileUrl: (url, options: FileImporterWithRequestContextOptions) =>
122+
findFileUrl: (url, options) =>
143123
resolutionCache.getOrCreate(url, async () => {
144124
const result = await resolveUrl(url, options);
145125
if (result.path) {

packages/angular_devkit/build_angular/src/tools/sass/rebasing-importer.ts

+3-68
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { readFileSync, readdirSync } from 'node:fs';
1212
import { basename, dirname, extname, join, relative } from 'node:path';
1313
import { fileURLToPath, pathToFileURL } from 'node:url';
1414
import type { CanonicalizeContext, Importer, ImporterResult, Syntax } from 'sass';
15-
import { findImports, findUrls } from './lexer';
15+
import { findUrls } from './lexer';
1616

1717
/**
1818
* A preprocessed cache entry for the files and directories within a previously searched
@@ -23,45 +23,6 @@ export interface DirectoryEntry {
2323
directories: Set<string>;
2424
}
2525

26-
/**
27-
* A prefix that is added to import and use directive specifiers that should be resolved
28-
* as modules and that will contain added resolve directory information.
29-
*
30-
* This functionality is used to workaround the Sass limitation that it does not provide the
31-
* importer file to custom resolution plugins.
32-
*/
33-
const MODULE_RESOLUTION_PREFIX = '__NG_PACKAGE__';
34-
35-
function packModuleSpecifier(specifier: string, resolveDir: string): string {
36-
const packed =
37-
MODULE_RESOLUTION_PREFIX +
38-
';' +
39-
// Encode the resolve directory to prevent unsupported characters from being present when
40-
// Sass processes the URL. This is important on Windows which can contain drive letters
41-
// and colons which would otherwise be interpreted as a URL scheme.
42-
encodeURIComponent(resolveDir) +
43-
';' +
44-
// Escape characters instead of encoding to provide more friendly not found error messages.
45-
// Unescaping is automatically handled by Sass.
46-
// https://developer.mozilla.org/en-US/docs/Web/CSS/url#syntax
47-
specifier.replace(/[()\s'"]/g, '\\$&');
48-
49-
return packed;
50-
}
51-
52-
function unpackModuleSpecifier(specifier: string): { specifier: string; resolveDir?: string } {
53-
if (!specifier.startsWith(`${MODULE_RESOLUTION_PREFIX};`)) {
54-
return { specifier };
55-
}
56-
57-
const values = specifier.split(';', 3);
58-
59-
return {
60-
specifier: values[2],
61-
resolveDir: decodeURIComponent(values[1]),
62-
};
63-
}
64-
6526
/**
6627
* A Sass Importer base class that provides the load logic to rebase all `url()` functions
6728
* within a stylesheet. The rebasing will ensure that the URLs in the output of the Sass compiler
@@ -114,27 +75,6 @@ abstract class UrlRebasingImporter implements Importer<'sync'> {
11475
updatedContents.update(start, end, rebasedUrl);
11576
}
11677

117-
// Add resolution directory information to module specifiers to facilitate resolution
118-
for (const { start, end, specifier } of findImports(contents)) {
119-
// Currently only provide directory information for known/common packages:
120-
// * `@material/`
121-
// * `@angular/`
122-
//
123-
// Comprehensive pre-resolution support may be added in the future. This is complicated by CSS/Sass not
124-
// requiring a `./` or `../` prefix to signify relative paths. A bare specifier could be either relative
125-
// or a module specifier. To differentiate, a relative resolution would need to be attempted first.
126-
if (!specifier.startsWith('@angular/') && !specifier.startsWith('@material/')) {
127-
continue;
128-
}
129-
130-
updatedContents ??= new MagicString(contents);
131-
updatedContents.update(
132-
start,
133-
end,
134-
`"${packModuleSpecifier(specifier, stylesheetDirectory)}"`,
135-
);
136-
}
137-
13878
if (updatedContents) {
13979
contents = updatedContents.toString();
14080
if (this.rebaseSourceMaps) {
@@ -348,10 +288,7 @@ export class ModuleUrlRebasingImporter extends RelativeUrlRebasingImporter {
348288
entryDirectory: string,
349289
directoryCache: Map<string, DirectoryEntry>,
350290
rebaseSourceMaps: Map<string, RawSourceMap> | undefined,
351-
private finder: (
352-
specifier: string,
353-
options: CanonicalizeContext & { resolveDir?: string },
354-
) => URL | null,
291+
private finder: (specifier: string, options: CanonicalizeContext) => URL | null,
355292
) {
356293
super(entryDirectory, directoryCache, rebaseSourceMaps);
357294
}
@@ -361,9 +298,7 @@ export class ModuleUrlRebasingImporter extends RelativeUrlRebasingImporter {
361298
return super.canonicalize(url, options);
362299
}
363300

364-
const { specifier, resolveDir } = unpackModuleSpecifier(url);
365-
366-
let result = this.finder(specifier, { ...options, resolveDir });
301+
let result = this.finder(url, options);
367302
result &&= super.canonicalize(result.href, options);
368303

369304
return result;

packages/angular_devkit/build_angular/src/tools/sass/sass-service.ts

+8-28
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,11 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import { dirname, join } from 'node:path';
9+
import { join } from 'node:path';
1010
import { fileURLToPath, pathToFileURL } from 'node:url';
1111
import { MessageChannel, Worker } from 'node:worker_threads';
1212
import {
13+
CanonicalizeContext,
1314
CompileResult,
1415
Exception,
1516
FileImporter,
@@ -31,24 +32,6 @@ const MAX_RENDER_WORKERS = maxWorkers;
3132
*/
3233
type RenderCallback = (error?: Exception, result?: CompileResult) => void;
3334

34-
type FileImporterOptions = Parameters<FileImporter['findFileUrl']>[1];
35-
36-
export interface FileImporterWithRequestContextOptions extends FileImporterOptions {
37-
/**
38-
* This is a custom option and is required as SASS does not provide context from which the file is being resolved.
39-
* This breaks Yarn PNP as transitive deps cannot be resolved from the workspace root.
40-
*
41-
* Workaround until https://github.com/sass/sass/issues/3247 is addressed.
42-
*/
43-
previousResolvedModules?: Set<string>;
44-
45-
/**
46-
* The base directory to use when resolving the request.
47-
* This value is only set if using the rebasing importers.
48-
*/
49-
resolveDir?: string;
50-
}
51-
5235
/**
5336
* An object containing the contextual information for a specific render request.
5437
*/
@@ -58,7 +41,6 @@ interface RenderRequest {
5841
callback: RenderCallback;
5942
logger?: Logger;
6043
importers?: Importers[];
61-
previousResolvedModules?: Set<string>;
6244
}
6345

6446
/**
@@ -244,7 +226,7 @@ export class SassWorkerImplementation {
244226

245227
mainImporterPort.on(
246228
'message',
247-
({ id, url, options }: { id: number; url: string; options: FileImporterOptions }) => {
229+
({ id, url, options }: { id: number; url: string; options: CanonicalizeContext }) => {
248230
const request = this.requests.get(id);
249231
if (!request?.importers) {
250232
mainImporterPort.postMessage(null);
@@ -256,14 +238,12 @@ export class SassWorkerImplementation {
256238

257239
this.processImporters(request.importers, url, {
258240
...options,
259-
previousResolvedModules: request.previousResolvedModules,
241+
// URL is not serializable so in the worker we convert to string and here back to URL.
242+
containingUrl: options.containingUrl
243+
? pathToFileURL(options.containingUrl as unknown as string)
244+
: null,
260245
})
261246
.then((result) => {
262-
if (result) {
263-
request.previousResolvedModules ??= new Set();
264-
request.previousResolvedModules.add(dirname(result));
265-
}
266-
267247
mainImporterPort.postMessage(result);
268248
})
269249
.catch((error) => {
@@ -284,7 +264,7 @@ export class SassWorkerImplementation {
284264
private async processImporters(
285265
importers: Iterable<Importers>,
286266
url: string,
287-
options: FileImporterWithRequestContextOptions,
267+
options: CanonicalizeContext,
288268
): Promise<string | null> {
289269
for (const importer of importers) {
290270
if (this.isImporter(importer)) {

packages/angular_devkit/build_angular/src/tools/sass/worker.ts

+8-1
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,14 @@ parentPort.on('message', (message: RenderRequestMessage) => {
9393
const proxyImporter: FileImporter<'sync'> = {
9494
findFileUrl: (url, options) => {
9595
Atomics.store(importerSignal, 0, 0);
96-
workerImporterPort.postMessage({ id, url, options });
96+
workerImporterPort.postMessage({
97+
id,
98+
url,
99+
options: {
100+
...options,
101+
containingUrl: options.containingUrl ? fileURLToPath(options.containingUrl) : null,
102+
},
103+
});
97104
Atomics.wait(importerSignal, 0, 0);
98105

99106
const result = receiveMessageOnPort(workerImporterPort)?.message as string | null;

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

+9-20
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,13 @@
88

99
import MiniCssExtractPlugin from 'mini-css-extract-plugin';
1010
import * as path from 'node:path';
11-
import { pathToFileURL } from 'node:url';
11+
import { fileURLToPath, pathToFileURL } from 'node:url';
1212
import type { FileImporter } from 'sass';
1313
import type { Configuration, LoaderContext, RuleSetUseItem } from 'webpack';
1414
import { WebpackConfigOptions } from '../../../utils/build-options';
1515
import { useLegacySass } from '../../../utils/environment-options';
1616
import { findTailwindConfigurationFile } from '../../../utils/tailwind';
17-
import {
18-
FileImporterWithRequestContextOptions,
19-
SassWorkerImplementation,
20-
} from '../../sass/sass-service';
17+
import { SassWorkerImplementation } from '../../sass/sass-service';
2118
import { SassLegacyWorkerImplementation } from '../../sass/sass-service-legacy';
2219
import {
2320
AnyComponentStyleBudgetChecker,
@@ -405,28 +402,20 @@ function getSassResolutionImporter(
405402
});
406403

407404
return {
408-
findFileUrl: async (
409-
url,
410-
{ fromImport, previousResolvedModules }: FileImporterWithRequestContextOptions,
411-
): Promise<URL | null> => {
405+
findFileUrl: async (url, { fromImport, containingUrl }): Promise<URL | null> => {
412406
if (url.charAt(0) === '.') {
413407
// Let Sass handle relative imports.
414408
return null;
415409
}
416410

411+
let resolveDir = root;
412+
if (containingUrl) {
413+
resolveDir = path.dirname(fileURLToPath(containingUrl));
414+
}
415+
417416
const resolve = fromImport ? resolveImport : resolveModule;
418417
// Try to resolve from root of workspace
419-
let result = await tryResolve(resolve, root, url);
420-
421-
// Try to resolve from previously resolved modules.
422-
if (!result && previousResolvedModules) {
423-
for (const path of previousResolvedModules) {
424-
result = await tryResolve(resolve, path, url);
425-
if (result) {
426-
break;
427-
}
428-
}
429-
}
418+
const result = await tryResolve(resolve, resolveDir, url);
430419

431420
return result ? pathToFileURL(result) : null;
432421
},

0 commit comments

Comments
 (0)