Skip to content

Commit

Permalink
Implement access tracking for containingUrl (#285)
Browse files Browse the repository at this point in the history
  • Loading branch information
ntkme committed Apr 17, 2024
1 parent fd32e8e commit 3080e76
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 21 deletions.
32 changes: 32 additions & 0 deletions 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;
}
}
27 changes: 15 additions & 12 deletions lib/src/importer-registry.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -115,21 +116,22 @@ export class ImporterRegistry<sync extends 'sync' | 'async'> {
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,
})
);
},
Expand Down Expand Up @@ -197,15 +199,15 @@ export class ImporterRegistry<sync extends 'sync' | 'async'> {
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:') {
Expand All @@ -216,6 +218,7 @@ export class ImporterRegistry<sync extends 'sync' | 'async'> {
}
return new proto.InboundMessage_FileImportResponse({
result: {case: 'fileUrl', value: url.toString()},
containingUrlUnused: !canonicalizeContext.containingUrlAccessed,
});
}
);
Expand Down
26 changes: 25 additions & 1 deletion lib/src/legacy/importer.ts
Expand Up @@ -86,10 +86,34 @@ export class LegacyImporterWrapper<sync extends 'sync' | 'async'>

canonicalize(
url: string,
options: {fromImport: boolean}
options: {fromImport: boolean; containingUrl: URL | null}
): PromiseOr<URL | null, sync> {
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)
Expand Down
19 changes: 14 additions & 5 deletions lib/src/legacy/index.ts
Expand Up @@ -184,11 +184,20 @@ function convertStringOptions<sync extends 'sync' | 'async'>(
): StringOptions<sync> & {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<sync> | FileImporter<sync>;
// 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,
Expand Down
8 changes: 6 additions & 2 deletions lib/src/value/argument-list.ts
Expand Up @@ -31,17 +31,21 @@ export class SassArgumentList extends SassList {
*/
readonly keywordsInternal: OrderedMap<string, Value>;

private _keywordsAccessed = false;

/**
* Whether the `keywords` getter has been accessed.
*
* This is marked as public so that the protofier 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.
*/
keywordsAccessed = false;
get keywordsAccessed(): boolean {
return this._keywordsAccessed;
}

get keywords(): OrderedMap<string, Value> {
this.keywordsAccessed = true;
this._keywordsAccessed = true;
return this.keywordsInternal;
}

Expand Down
2 changes: 1 addition & 1 deletion 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",
Expand Down

0 comments on commit 3080e76

Please sign in to comment.