From 24f095b184015a3369349c321f34e0f8b2455655 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Thu, 11 Apr 2024 21:03:15 -0700 Subject: [PATCH 1/2] Implement access tracking for containingUrl --- lib/src/canonicalize-context.ts | 32 ++++++++++++++++++++++++++++++++ lib/src/importer-registry.ts | 27 +++++++++++++++------------ lib/src/value/argument-list.ts | 8 ++++++-- package.json | 2 +- 4 files changed, 54 insertions(+), 15 deletions(-) create mode 100644 lib/src/canonicalize-context.ts diff --git a/lib/src/canonicalize-context.ts b/lib/src/canonicalize-context.ts new file mode 100644 index 00000000..46111a8a --- /dev/null +++ b/lib/src/canonicalize-context.ts @@ -0,0 +1,32 @@ +// Copyright 2024 Google LLC. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +export class CanonicalizeContext { + readonly fromImport: boolean; + + private readonly _containingUrl: URL | null; + + get containingUrl(): URL | null { + this._containingUrlAccessed = true; + return this._containingUrl; + } + + private _containingUrlAccessed = false; + + /** + * Whether the `containingUrl` getter has been accessed. + * + * This is marked as public so that the importer registry can access it, but + * it's not part of the package's public API and should not be accessed by + * user code. It may be renamed or removed without warning in the future. + */ + get containingUrlAccessed(): boolean { + return this._containingUrlAccessed; + } + + constructor(containingUrl: URL | null, fromImport: boolean) { + this._containingUrl = containingUrl; + this.fromImport = fromImport; + } +} diff --git a/lib/src/importer-registry.ts b/lib/src/importer-registry.ts index 9a1cb337..0f8bdc22 100644 --- a/lib/src/importer-registry.ts +++ b/lib/src/importer-registry.ts @@ -7,6 +7,7 @@ import * as p from 'path'; import {URL} from 'url'; import {inspect} from 'util'; +import {CanonicalizeContext} from './canonicalize-context'; import * as utils from './utils'; import {FileImporter, Importer, Options} from './vendor/sass'; import * as proto from './vendor/embedded_sass_pb'; @@ -115,21 +116,22 @@ export class ImporterRegistry { throw utils.compilerError('Unknown CanonicalizeRequest.importer_id'); } + const canonicalizeContext = new CanonicalizeContext( + request.containingUrl ? new URL(request.containingUrl) : null, + request.fromImport + ); + return catchOr( () => { return thenOr( - importer.canonicalize(request.url, { - fromImport: request.fromImport, - containingUrl: request.containingUrl - ? new URL(request.containingUrl) - : null, - }), + importer.canonicalize(request.url, canonicalizeContext), url => new proto.InboundMessage_CanonicalizeResponse({ result: url === null ? {case: undefined} : {case: 'url', value: url.toString()}, + containingUrlUnused: !canonicalizeContext.containingUrlAccessed, }) ); }, @@ -197,15 +199,15 @@ export class ImporterRegistry { throw utils.compilerError('Unknown FileImportRequest.importer_id'); } + const canonicalizeContext = new CanonicalizeContext( + request.containingUrl ? new URL(request.containingUrl) : null, + request.fromImport + ); + return catchOr( () => { return thenOr( - importer.findFileUrl(request.url, { - fromImport: request.fromImport, - containingUrl: request.containingUrl - ? new URL(request.containingUrl) - : null, - }), + importer.findFileUrl(request.url, canonicalizeContext), url => { if (!url) return new proto.InboundMessage_FileImportResponse(); if (url.protocol !== 'file:') { @@ -216,6 +218,7 @@ export class ImporterRegistry { } return new proto.InboundMessage_FileImportResponse({ result: {case: 'fileUrl', value: url.toString()}, + containingUrlUnused: !canonicalizeContext.containingUrlAccessed, }); } ); diff --git a/lib/src/value/argument-list.ts b/lib/src/value/argument-list.ts index e5c41cb1..b898796a 100644 --- a/lib/src/value/argument-list.ts +++ b/lib/src/value/argument-list.ts @@ -31,6 +31,8 @@ export class SassArgumentList extends SassList { */ readonly keywordsInternal: OrderedMap; + private _keywordsAccessed = false; + /** * Whether the `keywords` getter has been accessed. * @@ -38,10 +40,12 @@ export class SassArgumentList extends SassList { * part of the package's public API and should not be accessed by user code. * It may be renamed or removed without warning in the future. */ - keywordsAccessed = false; + get keywordsAccessed(): boolean { + return this._keywordsAccessed; + } get keywords(): OrderedMap { - this.keywordsAccessed = true; + this._keywordsAccessed = true; return this.keywordsInternal; } diff --git a/package.json b/package.json index 21539aba..9f82ef44 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "sass-embedded", "version": "1.75.0", - "protocol-version": "2.6.0", + "protocol-version": "2.7.0", "compiler-version": "1.75.0", "description": "Node.js library that communicates with Embedded Dart Sass using the Embedded Sass protocol", "repository": "sass/embedded-host-node", From 00a7a261bcf4438b06984879185e0e7c02da0bd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Tue, 16 Apr 2024 21:49:21 -0700 Subject: [PATCH 2/2] Emulate base importer inside LegacyImporterWrapper to track access to containingUrl --- lib/src/legacy/importer.ts | 26 +++++++++++++++++++++++++- lib/src/legacy/index.ts | 19 ++++++++++++++----- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/lib/src/legacy/importer.ts b/lib/src/legacy/importer.ts index 97ef9117..5977a3d5 100644 --- a/lib/src/legacy/importer.ts +++ b/lib/src/legacy/importer.ts @@ -86,10 +86,34 @@ export class LegacyImporterWrapper canonicalize( url: string, - options: {fromImport: boolean} + options: {fromImport: boolean; containingUrl: URL | null} ): PromiseOr { if (url.startsWith(endOfLoadProtocol)) return new URL(url); + // Emulate a base importer instead of using a real base importer, + // because we want to mark containingUrl as used, which is impossible + // in a real base importer. + if (options.containingUrl !== null) { + try { + const absoluteUrl = new URL(url, options.containingUrl).toString(); + const resolved = this.canonicalize(absoluteUrl, { + fromImport: options.fromImport, + containingUrl: null, + }); + if (resolved !== null) return resolved; + } catch (error: unknown) { + if ( + error instanceof TypeError && + isErrnoException(error) && + error.code === 'ERR_INVALID_URL' + ) { + // ignore + } else { + throw error; + } + } + } + if ( url.startsWith(legacyImporterProtocolPrefix) || url.startsWith(legacyImporterProtocol) diff --git a/lib/src/legacy/index.ts b/lib/src/legacy/index.ts index 404937cf..d7543e35 100644 --- a/lib/src/legacy/index.ts +++ b/lib/src/legacy/index.ts @@ -184,11 +184,20 @@ function convertStringOptions( ): StringOptions & {legacy: true} { const modernOptions = convertOptions(options, sync); - // Find the first non-NodePackageImporter to pass as legacy `importer` option. - // NodePackageImporter will be passed in `modernOptions.importers`. - const importer = modernOptions.importers?.find( - _importer => !(_importer instanceof NodePackageImporter) - ) as Importer | FileImporter; + // Use a no-op base importer, because the LegacyImporterWrapper will emulate + // the base importer by itself in order to mark containingUrl as accessed. + const importer = modernOptions.importers?.some( + importer => importer instanceof LegacyImporterWrapper + ) + ? { + canonicalize() { + return null; + }, + load() { + return null; + }, + } + : undefined; return { ...modernOptions,