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

Implement access tracking for containingUrl #2220

Merged
merged 3 commits into from Apr 17, 2024
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
18 changes: 9 additions & 9 deletions lib/src/async_import_cache.dart
Expand Up @@ -11,6 +11,7 @@ import 'package:path/path.dart' as p;
import 'ast/sass.dart';
import 'deprecation.dart';
import 'importer.dart';
import 'importer/canonicalize_context.dart';
import 'importer/no_op.dart';
import 'importer/utils.dart';
import 'io.dart';
Expand Down Expand Up @@ -206,18 +207,17 @@ final class AsyncImportCache {
/// that result is cacheable at all.
Future<(AsyncCanonicalizeResult?, bool cacheable)> _canonicalize(
AsyncImporter importer, Uri url, Uri? baseUrl, bool forImport) async {
var canonicalize = forImport
? () => inImportRule(() => importer.canonicalize(url))
: () => importer.canonicalize(url);

var passContainingUrl = baseUrl != null &&
(url.scheme == '' || await importer.isNonCanonicalScheme(url.scheme));
var result = await withContainingUrl(
passContainingUrl ? baseUrl : null, canonicalize);

// TODO(sass/dart-sass#2208): Determine whether the containing URL was
// _actually_ accessed rather than assuming it was.
var cacheable = !passContainingUrl || importer is FilesystemImporter;
var canonicalizeContext =
CanonicalizeContext(passContainingUrl ? baseUrl : null, forImport);

var result = await withCanonicalizeContext(
canonicalizeContext, () => importer.canonicalize(url));

var cacheable =
!passContainingUrl || !canonicalizeContext.wasContainingUrlAccessed;

if (result == null) return (null, cacheable);

Expand Down
4 changes: 3 additions & 1 deletion lib/src/embedded/importer/file.dart
Expand Up @@ -21,10 +21,12 @@ final class FileImporter extends ImporterBase {
..importerId = _importerId
..url = url.toString()
..fromImport = fromImport;
if (containingUrl case var containingUrl?) {
if (canonicalizeContext.containingUrlWithoutMarking
case var containingUrl?) {
request.containingUrl = containingUrl.toString();
}
var response = dispatcher.sendFileImportRequest(request);
if (!response.containingUrlUnused) canonicalizeContext.containingUrl;

switch (response.whichResult()) {
case InboundMessage_FileImportResponse_Result.fileUrl:
Expand Down
4 changes: 3 additions & 1 deletion lib/src/embedded/importer/host.dart
Expand Up @@ -35,10 +35,12 @@ final class HostImporter extends ImporterBase {
..importerId = _importerId
..url = url.toString()
..fromImport = fromImport;
if (containingUrl case var containingUrl?) {
if (canonicalizeContext.containingUrlWithoutMarking
case var containingUrl?) {
request.containingUrl = containingUrl.toString();
}
var response = dispatcher.sendCanonicalizeRequest(request);
if (!response.containingUrlUnused) canonicalizeContext.containingUrl;

return switch (response.whichResult()) {
InboundMessage_CanonicalizeResponse_Result.url =>
Expand Down
20 changes: 10 additions & 10 deletions lib/src/import_cache.dart
Expand Up @@ -5,7 +5,7 @@
// DO NOT EDIT. This file was generated from async_import_cache.dart.
// See tool/grind/synchronize.dart for details.
//
// Checksum: 37dd173d676ec6cf201a25b3cca9ac81d92b1433
// Checksum: 36bc42050cf2eb3a43f36376c4f06c1708eee777
//
// ignore_for_file: unused_import

Expand All @@ -18,6 +18,7 @@ import 'package:path/path.dart' as p;
import 'ast/sass.dart';
import 'deprecation.dart';
import 'importer.dart';
import 'importer/canonicalize_context.dart';
import 'importer/no_op.dart';
import 'importer/utils.dart';
import 'io.dart';
Expand Down Expand Up @@ -206,18 +207,17 @@ final class ImportCache {
/// that result is cacheable at all.
(CanonicalizeResult?, bool cacheable) _canonicalize(
Importer importer, Uri url, Uri? baseUrl, bool forImport) {
var canonicalize = forImport
? () => inImportRule(() => importer.canonicalize(url))
: () => importer.canonicalize(url);

var passContainingUrl = baseUrl != null &&
(url.scheme == '' || importer.isNonCanonicalScheme(url.scheme));
var result =
withContainingUrl(passContainingUrl ? baseUrl : null, canonicalize);

// TODO(sass/dart-sass#2208): Determine whether the containing URL was
// _actually_ accessed rather than assuming it was.
var cacheable = !passContainingUrl || importer is FilesystemImporter;
var canonicalizeContext =
CanonicalizeContext(passContainingUrl ? baseUrl : null, forImport);

var result = withCanonicalizeContext(
canonicalizeContext, () => importer.canonicalize(url));

var cacheable =
!passContainingUrl || !canonicalizeContext.wasContainingUrlAccessed;

if (result == null) return (null, cacheable);

Expand Down
15 changes: 14 additions & 1 deletion lib/src/importer/async.dart
Expand Up @@ -6,6 +6,7 @@ import 'dart:async';

import 'package:meta/meta.dart';

import 'canonicalize_context.dart';
import 'result.dart';
import 'utils.dart' as utils;

Expand Down Expand Up @@ -54,7 +55,19 @@ abstract class AsyncImporter {
/// Outside of that context, its value is undefined and subject to change.
@protected
@nonVirtual
Uri? get containingUrl => utils.containingUrl;
Uri? get containingUrl => utils.canonicalizeContext.containingUrl;

/// The canonicalize context of the stylesheet that caused the current
/// [canonicalize] invocation.
///
/// Subclasses should only access this from within calls to [canonicalize].
/// Outside of that context, its value is undefined and subject to change.
///
/// @nodoc
@internal
@protected
@nonVirtual
CanonicalizeContext get canonicalizeContext => utils.canonicalizeContext;

/// If [url] is recognized by this importer, returns its canonical format.
///
Expand Down
47 changes: 47 additions & 0 deletions lib/src/importer/canonicalize_context.dart
@@ -0,0 +1,47 @@
// Copyright 2024 Google Inc. 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.

import 'dart:async';

import 'package:meta/meta.dart';

/// Contextual information used by importers' `canonicalize` method.
@internal
final class CanonicalizeContext {
/// Whether the Sass compiler is currently evaluating an `@import` rule.
bool get fromImport => _fromImport;
bool _fromImport;

/// The URL of the stylesheet that contains the current load.
Uri? get containingUrl {
_wasContainingUrlAccessed = true;
return _containingUrl;
}

final Uri? _containingUrl;

/// Returns the same value as [containingUrl], but doesn't mark it accessed.
Uri? get containingUrlWithoutMarking => _containingUrl;

/// Whether [containingUrl] has been accessed.
///
/// This is used to determine whether canonicalize result is cacheable.
bool get wasContainingUrlAccessed => _wasContainingUrlAccessed;
var _wasContainingUrlAccessed = false;

/// Runs [callback] in a context with specificed [fromImport].
T withFromImport<T>(bool fromImport, T callback()) {
nex3 marked this conversation as resolved.
Show resolved Hide resolved
assert(Zone.current[#_canonicalizeContext] == this);

var oldFromImport = _fromImport;
_fromImport = fromImport;
try {
return callback();
} finally {
_fromImport = oldFromImport;
}
}

CanonicalizeContext(this._containingUrl, this._fromImport);
}
8 changes: 3 additions & 5 deletions lib/src/importer/js_to_dart/async.dart
Expand Up @@ -13,6 +13,7 @@ import '../../js/url.dart';
import '../../js/utils.dart';
import '../../util/nullable.dart';
import '../async.dart';
import '../canonicalize_context.dart';
import '../result.dart';
import 'utils.dart';

Expand All @@ -38,11 +39,8 @@ final class JSToDartAsyncImporter extends AsyncImporter {
}

FutureOr<Uri?> canonicalize(Uri url) async {
var result = wrapJSExceptions(() => _canonicalize(
url.toString(),
CanonicalizeContext(
fromImport: fromImport,
containingUrl: containingUrl.andThen(dartToJSUrl))));
var result = wrapJSExceptions(
() => _canonicalize(url.toString(), canonicalizeContext));
if (isPromise(result)) result = await promiseToFuture(result as Promise);
if (result == null) return null;

Expand Down
10 changes: 3 additions & 7 deletions lib/src/importer/js_to_dart/async_file.dart
Expand Up @@ -8,11 +8,10 @@ import 'package:cli_pkg/js.dart';
import 'package:node_interop/js.dart';
import 'package:node_interop/util.dart';

import '../../js/importer.dart';
import '../../js/url.dart';
import '../../js/utils.dart';
import '../../util/nullable.dart';
import '../async.dart';
import '../canonicalize_context.dart';
import '../filesystem.dart';
import '../result.dart';
import '../utils.dart';
Expand All @@ -28,11 +27,8 @@ final class JSToDartAsyncFileImporter extends AsyncImporter {
FutureOr<Uri?> canonicalize(Uri url) async {
if (url.scheme == 'file') return FilesystemImporter.cwd.canonicalize(url);

var result = wrapJSExceptions(() => _findFileUrl(
url.toString(),
CanonicalizeContext(
fromImport: fromImport,
containingUrl: containingUrl.andThen(dartToJSUrl))));
var result = wrapJSExceptions(
() => _findFileUrl(url.toString(), canonicalizeContext));
if (isPromise(result)) result = await promiseToFuture(result as Promise);
if (result == null) return null;
if (!isJSUrl(result)) {
Expand Down
10 changes: 3 additions & 7 deletions lib/src/importer/js_to_dart/file.dart
Expand Up @@ -6,10 +6,9 @@ import 'package:cli_pkg/js.dart';
import 'package:node_interop/js.dart';

import '../../importer.dart';
import '../../js/importer.dart';
import '../../js/url.dart';
import '../../js/utils.dart';
import '../../util/nullable.dart';
import '../canonicalize_context.dart';
import '../utils.dart';

/// A wrapper for a potentially-asynchronous JS API file importer that exposes
Expand All @@ -23,11 +22,8 @@ final class JSToDartFileImporter extends Importer {
Uri? canonicalize(Uri url) {
if (url.scheme == 'file') return FilesystemImporter.cwd.canonicalize(url);

var result = wrapJSExceptions(() => _findFileUrl(
url.toString(),
CanonicalizeContext(
fromImport: fromImport,
containingUrl: containingUrl.andThen(dartToJSUrl))));
var result = wrapJSExceptions(
() => _findFileUrl(url.toString(), canonicalizeContext));
if (result == null) return null;

if (isPromise(result)) {
Expand Down
8 changes: 3 additions & 5 deletions lib/src/importer/js_to_dart/sync.dart
Expand Up @@ -10,6 +10,7 @@ import '../../js/importer.dart';
import '../../js/url.dart';
import '../../js/utils.dart';
import '../../util/nullable.dart';
import '../canonicalize_context.dart';
import 'utils.dart';

/// A wrapper for a synchronous JS API importer that exposes it as a Dart
Expand All @@ -34,11 +35,8 @@ final class JSToDartImporter extends Importer {
}

Uri? canonicalize(Uri url) {
var result = wrapJSExceptions(() => _canonicalize(
url.toString(),
CanonicalizeContext(
fromImport: fromImport,
containingUrl: containingUrl.andThen(dartToJSUrl))));
var result = wrapJSExceptions(
() => _canonicalize(url.toString(), canonicalizeContext));
if (result == null) return null;
if (isJSUrl(result)) return jsToDartUrl(result as JSUrl);

Expand Down
38 changes: 22 additions & 16 deletions lib/src/importer/utils.dart
Expand Up @@ -7,6 +7,7 @@ import 'dart:async';
import 'package:path/path.dart' as p;

import '../io.dart';
import './canonicalize_context.dart';

/// Whether the Sass compiler is currently evaluating an `@import` rule.
///
Expand All @@ -15,30 +16,35 @@ import '../io.dart';
/// canonicalization should be identical for `@import` and `@use` rules. It's
/// admittedly hacky to set this globally, but `@import` will eventually be
/// removed, at which point we can delete this and have one consistent behavior.
bool get fromImport => Zone.current[#_inImportRule] as bool? ?? false;

/// The URL of the stylesheet that contains the current load.
Uri? get containingUrl => switch (Zone.current[#_containingUrl]) {
bool get fromImport =>
((Zone.current[#_canonicalizeContext] as CanonicalizeContext?)
?.fromImport ??
false);

/// The CanonicalizeContext of the current load.
CanonicalizeContext get canonicalizeContext =>
switch (Zone.current[#_canonicalizeContext]) {
null => throw StateError(
"containingUrl may only be accessed within a call to canonicalize()."),
#_none => null,
Uri url => url,
"canonicalizeContext may only be accessed within a call to canonicalize()."),
CanonicalizeContext context => context,
var value => throw StateError(
"Unexpected Zone.current[#_containingUrl] value $value.")
"Unexpected Zone.current[#_canonicalizeContext] value $value.")
};

/// Runs [callback] in a context where [fromImport] returns `true` and
/// [resolveImportPath] uses `@import` semantics rather than `@use` semantics.
T inImportRule<T>(T callback()) =>
runZoned(callback, zoneValues: {#_inImportRule: true});
switch (Zone.current[#_canonicalizeContext]) {
null => runZoned(callback,
zoneValues: {#_canonicalizeContext: CanonicalizeContext(null, true)}),
CanonicalizeContext context => context.withFromImport(true, callback),
var value => throw StateError(
"Unexpected Zone.current[#_canonicalizeContext] value $value.")
};

/// Runs [callback] in a context where [containingUrl] returns [url].
///
/// If [when] is `false`, runs [callback] without setting [containingUrl].
T withContainingUrl<T>(Uri? url, T callback()) =>
// Use #_none as a sentinel value so we can distinguish a containing URL
// that's set to null from one that's unset at all.
runZoned(callback, zoneValues: {#_containingUrl: url ?? #_none});
/// Runs [callback] in the given context.
T withCanonicalizeContext<T>(CanonicalizeContext? context, T callback()) =>
runZoned(callback, zoneValues: {#_canonicalizeContext: context});

/// Resolves an imported path using the same logic as the filesystem importer.
///
Expand Down
2 changes: 2 additions & 0 deletions lib/src/js.dart
Expand Up @@ -7,6 +7,7 @@ import 'package:js/js_util.dart';
import 'js/exception.dart';
import 'js/deprecations.dart';
import 'js/exports.dart';
import 'js/importer/canonicalize_context.dart';
import 'js/compile.dart';
import 'js/compiler.dart';
import 'js/legacy.dart';
Expand Down Expand Up @@ -64,6 +65,7 @@ void main() {
"dart2js\t${const String.fromEnvironment('dart-version')}\t"
"(Dart Compiler)\t[Dart]";

updateCanonicalizeContextPrototype();
updateSourceSpanPrototype();

// Legacy API
Expand Down
10 changes: 1 addition & 9 deletions lib/src/js/importer.dart
Expand Up @@ -4,6 +4,7 @@

import 'package:js/js.dart';

import '../importer/canonicalize_context.dart';
import 'url.dart';

@JS()
Expand All @@ -15,15 +16,6 @@ class JSImporter {
external Object? get nonCanonicalScheme;
}

@JS()
@anonymous
class CanonicalizeContext {
external bool get fromImport;
external JSUrl? get containingUrl;

external factory CanonicalizeContext({bool fromImport, JSUrl? containingUrl});
}

@JS()
@anonymous
class JSImporterResult {
Expand Down
16 changes: 16 additions & 0 deletions lib/src/js/importer/canonicalize_context.dart
@@ -0,0 +1,16 @@
// Copyright 2014 Google Inc. 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.

import '../../importer/canonicalize_context.dart';
import '../../util/nullable.dart';
import '../reflection.dart';
import '../utils.dart';

/// Adds JS members to Dart's `CanonicalizeContext` class.
void updateCanonicalizeContextPrototype() =>
getJSClass(CanonicalizeContext(null, false)).defineGetters({
'fromImport': (CanonicalizeContext self) => self.fromImport,
'containingUrl': (CanonicalizeContext self) =>
self.containingUrl.andThen(dartToJSUrl),
});