Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): resolve transitive dependencies i…
Browse files Browse the repository at this point in the history
…n Sass when using Yarn PNP

Enhanced resolver is unable to resolve transitive dependencies in Sass when using Yarn PNP. The main reason for this is that Sass doesn't provide context on which file is requesting the module. See: sass/sass#3247.

As a workaround for this we store previously resolved paths and when a new request comes in we try to resolve this from the previously resolved files if we are unable to resolve the request from the workspace root.

(cherry picked from commit c49f1ee)
  • Loading branch information
alan-agius4 committed Oct 14, 2022
1 parent 22955f2 commit 2105964
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 24 deletions.
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;
}

0 comments on commit 2105964

Please sign in to comment.