From 8b90f1078be5da4ff0715818186b624fb5d5b910 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 13:38:06 -0700 Subject: [PATCH] Implement access tracking for containingUrl --- lib/src/async_import_cache.dart | 25 ++++++++--------- lib/src/embedded/importer/host.dart | 6 ++++- lib/src/import_cache.dart | 26 +++++++++--------- lib/src/importer/async.dart | 12 ++++++++- lib/src/importer/canonicalize_context.dart | 24 +++++++++++++++++ lib/src/importer/js_to_dart/async.dart | 7 ++--- lib/src/importer/js_to_dart/async_file.dart | 8 ++---- lib/src/importer/js_to_dart/file.dart | 8 ++---- lib/src/importer/js_to_dart/sync.dart | 7 ++--- lib/src/importer/utils.dart | 21 ++++++++------- lib/src/js/importer.dart | 30 ++++++++++++++++++--- 11 files changed, 112 insertions(+), 62 deletions(-) create mode 100644 lib/src/importer/canonicalize_context.dart diff --git a/lib/src/async_import_cache.dart b/lib/src/async_import_cache.dart index 576094cb4..4a4386320 100644 --- a/lib/src/async_import_cache.dart +++ b/lib/src/async_import_cache.dart @@ -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'; @@ -206,18 +207,18 @@ 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( + baseUrl != null && + (url.scheme == '' || + await importer.isNonCanonicalScheme(url.scheme)) + ? baseUrl + : null, + forImport); + + var result = await withCanonicalizeContext( + canonicalizeContext, () => importer.canonicalize(url)); + + var cacheable = !canonicalizeContext.wasContainingUrlAccessed; if (result == null) return (null, cacheable); diff --git a/lib/src/embedded/importer/host.dart b/lib/src/embedded/importer/host.dart index 25245721b..e7f10d3ec 100644 --- a/lib/src/embedded/importer/host.dart +++ b/lib/src/embedded/importer/host.dart @@ -35,11 +35,15 @@ final class HostImporter extends ImporterBase { ..importerId = _importerId ..url = url.toString() ..fromImport = fromImport; - if (containingUrl case var containingUrl?) { + var containingUrl = canonicalizeContext.containingUrlWithoutMarking; + if (containingUrl != null) { request.containingUrl = containingUrl.toString(); } var response = dispatcher.sendCanonicalizeRequest(request); + // TODO: + if (true) canonicalizeContext.containingUrl; + return switch (response.whichResult()) { InboundMessage_CanonicalizeResponse_Result.url => parseAbsoluteUrl("The importer", response.url), diff --git a/lib/src/import_cache.dart b/lib/src/import_cache.dart index 6204971e4..dd6e76d81 100644 --- a/lib/src/import_cache.dart +++ b/lib/src/import_cache.dart @@ -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: 1a0d1c1f2a0ddf0d18b9a0af9636d99a58bec770 // // ignore_for_file: unused_import @@ -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'; @@ -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( + baseUrl != null && + (url.scheme == '' || importer.isNonCanonicalScheme(url.scheme)) + ? baseUrl + : null, + forImport); + + var result = withCanonicalizeContext( + canonicalizeContext, () => importer.canonicalize(url)); + + var cacheable = !canonicalizeContext.wasContainingUrlAccessed; if (result == null) return (null, cacheable); diff --git a/lib/src/importer/async.dart b/lib/src/importer/async.dart index d7d6951fe..00d29993a 100644 --- a/lib/src/importer/async.dart +++ b/lib/src/importer/async.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'package:meta/meta.dart'; +import 'canonicalize_context.dart'; import 'result.dart'; import 'utils.dart' as utils; @@ -54,7 +55,16 @@ 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. + @protected + @nonVirtual + CanonicalizeContext get canonicalizeContext => utils.canonicalizeContext; /// If [url] is recognized by this importer, returns its canonical format. /// diff --git a/lib/src/importer/canonicalize_context.dart b/lib/src/importer/canonicalize_context.dart new file mode 100644 index 000000000..f40a79229 --- /dev/null +++ b/lib/src/importer/canonicalize_context.dart @@ -0,0 +1,24 @@ +// 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 'package:meta/meta.dart'; + +class CanonicalizeContext { + final bool fromImport; + + Uri? get containingUrl { + _wasContainingUrlAccessed = true; + return _containingUrl; + } + + final Uri? _containingUrl; + + Uri? get containingUrlWithoutMarking => _containingUrl; + + @internal + bool get wasContainingUrlAccessed => _wasContainingUrlAccessed; + bool _wasContainingUrlAccessed = false; + + CanonicalizeContext(this._containingUrl, this.fromImport); +} diff --git a/lib/src/importer/js_to_dart/async.dart b/lib/src/importer/js_to_dart/async.dart index 11ffbd735..a11611a2b 100644 --- a/lib/src/importer/js_to_dart/async.dart +++ b/lib/src/importer/js_to_dart/async.dart @@ -20,7 +20,7 @@ import 'utils.dart'; /// a Dart [AsyncImporter]. final class JSToDartAsyncImporter extends AsyncImporter { /// The wrapped canonicalize function. - final Object? Function(String, CanonicalizeContext) _canonicalize; + final Object? Function(String, JSCanonicalizeContext) _canonicalize; /// The wrapped load function. final Object? Function(JSUrl) _load; @@ -39,10 +39,7 @@ final class JSToDartAsyncImporter extends AsyncImporter { FutureOr canonicalize(Uri url) async { var result = wrapJSExceptions(() => _canonicalize( - url.toString(), - CanonicalizeContext( - fromImport: fromImport, - containingUrl: containingUrl.andThen(dartToJSUrl)))); + url.toString(), createJSCanonicalizeContext(canonicalizeContext))); if (isPromise(result)) result = await promiseToFuture(result as Promise); if (result == null) return null; diff --git a/lib/src/importer/js_to_dart/async_file.dart b/lib/src/importer/js_to_dart/async_file.dart index 7be4b9461..40ff300c8 100644 --- a/lib/src/importer/js_to_dart/async_file.dart +++ b/lib/src/importer/js_to_dart/async_file.dart @@ -11,7 +11,6 @@ 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 '../filesystem.dart'; import '../result.dart'; @@ -21,7 +20,7 @@ import '../utils.dart'; /// it as a Dart [AsyncImporter]. final class JSToDartAsyncFileImporter extends AsyncImporter { /// The wrapped `findFileUrl` function. - final Object? Function(String, CanonicalizeContext) _findFileUrl; + final Object? Function(String, JSCanonicalizeContext) _findFileUrl; JSToDartAsyncFileImporter(this._findFileUrl); @@ -29,10 +28,7 @@ final class JSToDartAsyncFileImporter extends AsyncImporter { if (url.scheme == 'file') return FilesystemImporter.cwd.canonicalize(url); var result = wrapJSExceptions(() => _findFileUrl( - url.toString(), - CanonicalizeContext( - fromImport: fromImport, - containingUrl: containingUrl.andThen(dartToJSUrl)))); + url.toString(), createJSCanonicalizeContext(canonicalizeContext))); if (isPromise(result)) result = await promiseToFuture(result as Promise); if (result == null) return null; if (!isJSUrl(result)) { diff --git a/lib/src/importer/js_to_dart/file.dart b/lib/src/importer/js_to_dart/file.dart index e3302f881..04857508b 100644 --- a/lib/src/importer/js_to_dart/file.dart +++ b/lib/src/importer/js_to_dart/file.dart @@ -9,14 +9,13 @@ import '../../importer.dart'; import '../../js/importer.dart'; import '../../js/url.dart'; import '../../js/utils.dart'; -import '../../util/nullable.dart'; import '../utils.dart'; /// A wrapper for a potentially-asynchronous JS API file importer that exposes /// it as a Dart [AsyncImporter]. final class JSToDartFileImporter extends Importer { /// The wrapped `findFileUrl` function. - final Object? Function(String, CanonicalizeContext) _findFileUrl; + final Object? Function(String, JSCanonicalizeContext) _findFileUrl; JSToDartFileImporter(this._findFileUrl); @@ -24,10 +23,7 @@ final class JSToDartFileImporter extends Importer { if (url.scheme == 'file') return FilesystemImporter.cwd.canonicalize(url); var result = wrapJSExceptions(() => _findFileUrl( - url.toString(), - CanonicalizeContext( - fromImport: fromImport, - containingUrl: containingUrl.andThen(dartToJSUrl)))); + url.toString(), createJSCanonicalizeContext(canonicalizeContext))); if (result == null) return null; if (isPromise(result)) { diff --git a/lib/src/importer/js_to_dart/sync.dart b/lib/src/importer/js_to_dart/sync.dart index f69dafb35..ef1f52f27 100644 --- a/lib/src/importer/js_to_dart/sync.dart +++ b/lib/src/importer/js_to_dart/sync.dart @@ -16,7 +16,7 @@ import 'utils.dart'; /// [Importer]. final class JSToDartImporter extends Importer { /// The wrapped canonicalize function. - final Object? Function(String, CanonicalizeContext) _canonicalize; + final Object? Function(String, JSCanonicalizeContext) _canonicalize; /// The wrapped load function. final Object? Function(JSUrl) _load; @@ -35,10 +35,7 @@ final class JSToDartImporter extends Importer { Uri? canonicalize(Uri url) { var result = wrapJSExceptions(() => _canonicalize( - url.toString(), - CanonicalizeContext( - fromImport: fromImport, - containingUrl: containingUrl.andThen(dartToJSUrl)))); + url.toString(), createJSCanonicalizeContext(canonicalizeContext))); if (result == null) return null; if (isJSUrl(result)) return jsToDartUrl(result as JSUrl); diff --git a/lib/src/importer/utils.dart b/lib/src/importer/utils.dart index a68ae6f5e..f833fc599 100644 --- a/lib/src/importer/utils.dart +++ b/lib/src/importer/utils.dart @@ -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. /// @@ -15,16 +16,18 @@ 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; +bool get fromImport => (Zone.current[#_inImportRule] as bool? ?? + (Zone.current[#_canonicalizeContext] as CanonicalizeContext?)?.fromImport ?? + false); -/// The URL of the stylesheet that contains the current load. -Uri? get containingUrl => switch (Zone.current[#_containingUrl]) { +/// 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 @@ -35,10 +38,10 @@ T inImportRule(T callback()) => /// Runs [callback] in a context where [containingUrl] returns [url]. /// /// If [when] is `false`, runs [callback] without setting [containingUrl]. -T withContainingUrl(Uri? url, T callback()) => +T withCanonicalizeContext(CanonicalizeContext? context, 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}); + runZoned(callback, zoneValues: {#_canonicalizeContext: context}); /// Resolves an imported path using the same logic as the filesystem importer. /// diff --git a/lib/src/js/importer.dart b/lib/src/js/importer.dart index 0469737ee..18908e226 100644 --- a/lib/src/js/importer.dart +++ b/lib/src/js/importer.dart @@ -2,26 +2,48 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. +import 'dart:js_util'; + import 'package:js/js.dart'; +import '../importer/canonicalize_context.dart'; +import '../util/nullable.dart'; import 'url.dart'; +import 'utils.dart'; @JS() @anonymous class JSImporter { - external Object? Function(String, CanonicalizeContext)? get canonicalize; + external Object? Function(String, JSCanonicalizeContext)? get canonicalize; external Object? Function(JSUrl)? get load; - external Object? Function(String, CanonicalizeContext)? get findFileUrl; + external Object? Function(String, JSCanonicalizeContext)? get findFileUrl; external Object? get nonCanonicalScheme; } @JS() @anonymous -class CanonicalizeContext { +class JSCanonicalizeContext { external bool get fromImport; external JSUrl? get containingUrl; +} + +/// The use of `@JSExport()` together with `createDartExport` allow us +/// to create a JSObject that lazily access containingUrl +@JSExport() +class _JSExportCanonicalizeContext { + final CanonicalizeContext _canonicalizeContext; + + bool get fromImport => _canonicalizeContext.fromImport; + JSUrl? get containingUrl => + _canonicalizeContext.containingUrl.andThen(dartToJSUrl); + + _JSExportCanonicalizeContext(this._canonicalizeContext); +} - external factory CanonicalizeContext({bool fromImport, JSUrl? containingUrl}); +JSCanonicalizeContext createJSCanonicalizeContext( + CanonicalizeContext canonicalizeContext) { + return createDartExport(_JSExportCanonicalizeContext(canonicalizeContext)) + as JSCanonicalizeContext; } @JS()