Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(@angular-devkit/build-angular): resolve transitive dependencies in Sass when using Yarn PNP #24077

Merged
merged 1 commit into from Oct 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 24 additions & 5 deletions packages/angular_devkit/build_angular/src/sass/sass-service.ts
Expand Up @@ -6,7 +6,9 @@
* found in the LICENSE file at https://angular.io/license
*/

import { join } from 'path';
import { dirname, join } from 'node:path';
import { fileURLToPath, pathToFileURL } from 'node:url';
import { MessageChannel, Worker } from 'node:worker_threads';
import {
CompileResult,
Exception,
Expand All @@ -15,8 +17,6 @@ import {
StringOptionsWithImporter,
StringOptionsWithoutImporter,
} from 'sass';
import { fileURLToPath, pathToFileURL } from 'url';
import { MessageChannel, Worker } from 'worker_threads';
import { maxWorkers } from '../utils/environment-options';

/**
Expand All @@ -31,6 +31,16 @@ type RenderCallback = (error?: Exception, result?: CompileResult) => void;

type FileImporterOptions = Parameters<FileImporter['findFileUrl']>[1];

export interface FileImporterWithRequestContextOptions extends FileImporterOptions {
/**
* This is a custom option and is required as SASS does not provide context from which the file is being resolved.
* This breaks Yarn PNP as transitive deps cannot be resolved from the workspace root.
*
* Workaround until https://github.com/sass/sass/issues/3247 is addressed.
*/
previousResolvedModules?: Set<string>;
}

/**
* An object containing the contextual information for a specific render request.
*/
Expand All @@ -39,6 +49,7 @@ interface RenderRequest {
workerIndex: number;
callback: RenderCallback;
importers?: Importers[];
previousResolvedModules?: Set<string>;
}

/**
Expand Down Expand Up @@ -212,8 +223,16 @@ export class SassWorkerImplementation {
return;
}

this.processImporters(request.importers, url, options)
this.processImporters(request.importers, url, {
...options,
previousResolvedModules: request.previousResolvedModules,
})
.then((result) => {
if (result) {
request.previousResolvedModules ??= new Set();
request.previousResolvedModules.add(dirname(result));
}

mainImporterPort.postMessage(result);
})
.catch((error) => {
Expand All @@ -234,7 +253,7 @@ export class SassWorkerImplementation {
private async processImporters(
importers: Iterable<Importers>,
url: string,
options: FileImporterOptions,
options: FileImporterWithRequestContextOptions,
): Promise<string | null> {
for (const importer of importers) {
if (this.isImporter(importer)) {
Expand Down
64 changes: 45 additions & 19 deletions packages/angular_devkit/build_angular/src/webpack/configs/styles.ts
Expand Up @@ -6,13 +6,16 @@
* found in the LICENSE file at https://angular.io/license
*/

import * as fs from 'fs';
import MiniCssExtractPlugin from 'mini-css-extract-plugin';
import * as path from 'path';
import * as fs from 'node:fs';
import * as path from 'node:path';
import { pathToFileURL } from 'node:url';
import type { FileImporter } from 'sass';
import { pathToFileURL } from 'url';
import type { Configuration, LoaderContext, RuleSetUseItem } from 'webpack';
import { SassWorkerImplementation } from '../../sass/sass-service';
import {
FileImporterWithRequestContextOptions,
SassWorkerImplementation,
} from '../../sass/sass-service';
import { SassLegacyWorkerImplementation } from '../../sass/sass-service-legacy';
import { WebpackConfigOptions } from '../../utils/build-options';
import { useLegacySass } from '../../utils/environment-options';
Expand Down Expand Up @@ -413,30 +416,53 @@ function getSassResolutionImporter(
});

return {
findFileUrl: async (url, { fromImport }): Promise<URL | null> => {
findFileUrl: async (
url,
{ fromImport, previousResolvedModules }: FileImporterWithRequestContextOptions,
): Promise<URL | null> => {
if (url.charAt(0) === '.') {
// Let Sass handle relative imports.
return null;
}

let file: string | undefined;
const resolve = fromImport ? resolveImport : resolveModule;

try {
file = await resolve(root, url);
} catch {
// Try to resolve a partial file
// @use '@material/button/button' as mdc-button;
// `@material/button/button` -> `@material/button/_button`
const lastSlashIndex = url.lastIndexOf('/');
const underscoreIndex = lastSlashIndex + 1;
if (underscoreIndex > 0 && url.charAt(underscoreIndex) !== '_') {
const partialFileUrl = `${url.slice(0, underscoreIndex)}_${url.slice(underscoreIndex)}`;
file = await resolve(root, partialFileUrl).catch(() => undefined);
// Try to resolve from root of workspace
let result = await tryResolve(resolve, root, url);

// Try to resolve from previously resolved modules.
if (!result && previousResolvedModules) {
for (const path of previousResolvedModules) {
result = await tryResolve(resolve, path, url);
if (result) {
break;
}
}
}

return file ? pathToFileURL(file) : null;
return result ? pathToFileURL(result) : null;
},
};
}

async function tryResolve(
resolve: ReturnType<LoaderContext<{}>['getResolve']>,
root: string,
url: string,
): Promise<string | undefined> {
try {
return await resolve(root, url);
} catch {
// Try to resolve a partial file
// @use '@material/button/button' as mdc-button;
// `@material/button/button` -> `@material/button/_button`
const lastSlashIndex = url.lastIndexOf('/');
const underscoreIndex = lastSlashIndex + 1;
if (underscoreIndex > 0 && url.charAt(underscoreIndex) !== '_') {
const partialFileUrl = `${url.slice(0, underscoreIndex)}_${url.slice(underscoreIndex)}`;

return resolve(root, partialFileUrl).catch(() => undefined);
}
}

return undefined;
}