Skip to content

Commit 2a9eaad

Browse files
ntkmenex3
ntkme
and
nex3
authored
Implement access tracking for containingUrl (#2220)
Co-authored-by: Natalie Weizenbaum <[email protected]>
1 parent 821b98e commit 2a9eaad

15 files changed

+141
-72
lines changed

‎lib/src/async_import_cache.dart

+9-9
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import 'package:path/path.dart' as p;
1111
import 'ast/sass.dart';
1212
import 'deprecation.dart';
1313
import 'importer.dart';
14+
import 'importer/canonicalize_context.dart';
1415
import 'importer/no_op.dart';
1516
import 'importer/utils.dart';
1617
import 'io.dart';
@@ -206,18 +207,17 @@ final class AsyncImportCache {
206207
/// that result is cacheable at all.
207208
Future<(AsyncCanonicalizeResult?, bool cacheable)> _canonicalize(
208209
AsyncImporter importer, Uri url, Uri? baseUrl, bool forImport) async {
209-
var canonicalize = forImport
210-
? () => inImportRule(() => importer.canonicalize(url))
211-
: () => importer.canonicalize(url);
212-
213210
var passContainingUrl = baseUrl != null &&
214211
(url.scheme == '' || await importer.isNonCanonicalScheme(url.scheme));
215-
var result = await withContainingUrl(
216-
passContainingUrl ? baseUrl : null, canonicalize);
217212

218-
// TODO(sass/dart-sass#2208): Determine whether the containing URL was
219-
// _actually_ accessed rather than assuming it was.
220-
var cacheable = !passContainingUrl || importer is FilesystemImporter;
213+
var canonicalizeContext =
214+
CanonicalizeContext(passContainingUrl ? baseUrl : null, forImport);
215+
216+
var result = await withCanonicalizeContext(
217+
canonicalizeContext, () => importer.canonicalize(url));
218+
219+
var cacheable =
220+
!passContainingUrl || !canonicalizeContext.wasContainingUrlAccessed;
221221

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

‎lib/src/embedded/importer/file.dart

+3-1
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,12 @@ final class FileImporter extends ImporterBase {
2121
..importerId = _importerId
2222
..url = url.toString()
2323
..fromImport = fromImport;
24-
if (containingUrl case var containingUrl?) {
24+
if (canonicalizeContext.containingUrlWithoutMarking
25+
case var containingUrl?) {
2526
request.containingUrl = containingUrl.toString();
2627
}
2728
var response = dispatcher.sendFileImportRequest(request);
29+
if (!response.containingUrlUnused) canonicalizeContext.containingUrl;
2830

2931
switch (response.whichResult()) {
3032
case InboundMessage_FileImportResponse_Result.fileUrl:

‎lib/src/embedded/importer/host.dart

+3-1
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,12 @@ final class HostImporter extends ImporterBase {
3535
..importerId = _importerId
3636
..url = url.toString()
3737
..fromImport = fromImport;
38-
if (containingUrl case var containingUrl?) {
38+
if (canonicalizeContext.containingUrlWithoutMarking
39+
case var containingUrl?) {
3940
request.containingUrl = containingUrl.toString();
4041
}
4142
var response = dispatcher.sendCanonicalizeRequest(request);
43+
if (!response.containingUrlUnused) canonicalizeContext.containingUrl;
4244

4345
return switch (response.whichResult()) {
4446
InboundMessage_CanonicalizeResponse_Result.url =>

‎lib/src/import_cache.dart

+10-10
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
// DO NOT EDIT. This file was generated from async_import_cache.dart.
66
// See tool/grind/synchronize.dart for details.
77
//
8-
// Checksum: 37dd173d676ec6cf201a25b3cca9ac81d92b1433
8+
// Checksum: 36bc42050cf2eb3a43f36376c4f06c1708eee777
99
//
1010
// ignore_for_file: unused_import
1111

@@ -18,6 +18,7 @@ import 'package:path/path.dart' as p;
1818
import 'ast/sass.dart';
1919
import 'deprecation.dart';
2020
import 'importer.dart';
21+
import 'importer/canonicalize_context.dart';
2122
import 'importer/no_op.dart';
2223
import 'importer/utils.dart';
2324
import 'io.dart';
@@ -206,18 +207,17 @@ final class ImportCache {
206207
/// that result is cacheable at all.
207208
(CanonicalizeResult?, bool cacheable) _canonicalize(
208209
Importer importer, Uri url, Uri? baseUrl, bool forImport) {
209-
var canonicalize = forImport
210-
? () => inImportRule(() => importer.canonicalize(url))
211-
: () => importer.canonicalize(url);
212-
213210
var passContainingUrl = baseUrl != null &&
214211
(url.scheme == '' || importer.isNonCanonicalScheme(url.scheme));
215-
var result =
216-
withContainingUrl(passContainingUrl ? baseUrl : null, canonicalize);
217212

218-
// TODO(sass/dart-sass#2208): Determine whether the containing URL was
219-
// _actually_ accessed rather than assuming it was.
220-
var cacheable = !passContainingUrl || importer is FilesystemImporter;
213+
var canonicalizeContext =
214+
CanonicalizeContext(passContainingUrl ? baseUrl : null, forImport);
215+
216+
var result = withCanonicalizeContext(
217+
canonicalizeContext, () => importer.canonicalize(url));
218+
219+
var cacheable =
220+
!passContainingUrl || !canonicalizeContext.wasContainingUrlAccessed;
221221

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

‎lib/src/importer/async.dart

+14-1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'dart:async';
66

77
import 'package:meta/meta.dart';
88

9+
import 'canonicalize_context.dart';
910
import 'result.dart';
1011
import 'utils.dart' as utils;
1112

@@ -54,7 +55,19 @@ abstract class AsyncImporter {
5455
/// Outside of that context, its value is undefined and subject to change.
5556
@protected
5657
@nonVirtual
57-
Uri? get containingUrl => utils.containingUrl;
58+
Uri? get containingUrl => utils.canonicalizeContext.containingUrl;
59+
60+
/// The canonicalize context of the stylesheet that caused the current
61+
/// [canonicalize] invocation.
62+
///
63+
/// Subclasses should only access this from within calls to [canonicalize].
64+
/// Outside of that context, its value is undefined and subject to change.
65+
///
66+
/// @nodoc
67+
@internal
68+
@protected
69+
@nonVirtual
70+
CanonicalizeContext get canonicalizeContext => utils.canonicalizeContext;
5871

5972
/// If [url] is recognized by this importer, returns its canonical format.
6073
///

‎lib/src/importer/canonicalize_context.dart

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Copyright 2024 Google Inc. Use of this source code is governed by an
2+
// MIT-style license that can be found in the LICENSE file or at
3+
// https://opensource.org/licenses/MIT.
4+
5+
import 'dart:async';
6+
7+
import 'package:meta/meta.dart';
8+
9+
/// Contextual information used by importers' `canonicalize` method.
10+
@internal
11+
final class CanonicalizeContext {
12+
/// Whether the Sass compiler is currently evaluating an `@import` rule.
13+
bool get fromImport => _fromImport;
14+
bool _fromImport;
15+
16+
/// The URL of the stylesheet that contains the current load.
17+
Uri? get containingUrl {
18+
_wasContainingUrlAccessed = true;
19+
return _containingUrl;
20+
}
21+
22+
final Uri? _containingUrl;
23+
24+
/// Returns the same value as [containingUrl], but doesn't mark it accessed.
25+
Uri? get containingUrlWithoutMarking => _containingUrl;
26+
27+
/// Whether [containingUrl] has been accessed.
28+
///
29+
/// This is used to determine whether canonicalize result is cacheable.
30+
bool get wasContainingUrlAccessed => _wasContainingUrlAccessed;
31+
var _wasContainingUrlAccessed = false;
32+
33+
/// Runs [callback] in a context with specificed [fromImport].
34+
T withFromImport<T>(bool fromImport, T callback()) {
35+
assert(Zone.current[#_canonicalizeContext] == this);
36+
37+
var oldFromImport = _fromImport;
38+
_fromImport = fromImport;
39+
try {
40+
return callback();
41+
} finally {
42+
_fromImport = oldFromImport;
43+
}
44+
}
45+
46+
CanonicalizeContext(this._containingUrl, this._fromImport);
47+
}

‎lib/src/importer/js_to_dart/async.dart

+3-5
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import '../../js/url.dart';
1313
import '../../js/utils.dart';
1414
import '../../util/nullable.dart';
1515
import '../async.dart';
16+
import '../canonicalize_context.dart';
1617
import '../result.dart';
1718
import 'utils.dart';
1819

@@ -38,11 +39,8 @@ final class JSToDartAsyncImporter extends AsyncImporter {
3839
}
3940

4041
FutureOr<Uri?> canonicalize(Uri url) async {
41-
var result = wrapJSExceptions(() => _canonicalize(
42-
url.toString(),
43-
CanonicalizeContext(
44-
fromImport: fromImport,
45-
containingUrl: containingUrl.andThen(dartToJSUrl))));
42+
var result = wrapJSExceptions(
43+
() => _canonicalize(url.toString(), canonicalizeContext));
4644
if (isPromise(result)) result = await promiseToFuture(result as Promise);
4745
if (result == null) return null;
4846

‎lib/src/importer/js_to_dart/async_file.dart

+3-7
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,10 @@ import 'package:cli_pkg/js.dart';
88
import 'package:node_interop/js.dart';
99
import 'package:node_interop/util.dart';
1010

11-
import '../../js/importer.dart';
1211
import '../../js/url.dart';
1312
import '../../js/utils.dart';
14-
import '../../util/nullable.dart';
1513
import '../async.dart';
14+
import '../canonicalize_context.dart';
1615
import '../filesystem.dart';
1716
import '../result.dart';
1817
import '../utils.dart';
@@ -28,11 +27,8 @@ final class JSToDartAsyncFileImporter extends AsyncImporter {
2827
FutureOr<Uri?> canonicalize(Uri url) async {
2928
if (url.scheme == 'file') return FilesystemImporter.cwd.canonicalize(url);
3029

31-
var result = wrapJSExceptions(() => _findFileUrl(
32-
url.toString(),
33-
CanonicalizeContext(
34-
fromImport: fromImport,
35-
containingUrl: containingUrl.andThen(dartToJSUrl))));
30+
var result = wrapJSExceptions(
31+
() => _findFileUrl(url.toString(), canonicalizeContext));
3632
if (isPromise(result)) result = await promiseToFuture(result as Promise);
3733
if (result == null) return null;
3834
if (!isJSUrl(result)) {

‎lib/src/importer/js_to_dart/file.dart

+3-7
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,9 @@ import 'package:cli_pkg/js.dart';
66
import 'package:node_interop/js.dart';
77

88
import '../../importer.dart';
9-
import '../../js/importer.dart';
109
import '../../js/url.dart';
1110
import '../../js/utils.dart';
12-
import '../../util/nullable.dart';
11+
import '../canonicalize_context.dart';
1312
import '../utils.dart';
1413

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

26-
var result = wrapJSExceptions(() => _findFileUrl(
27-
url.toString(),
28-
CanonicalizeContext(
29-
fromImport: fromImport,
30-
containingUrl: containingUrl.andThen(dartToJSUrl))));
25+
var result = wrapJSExceptions(
26+
() => _findFileUrl(url.toString(), canonicalizeContext));
3127
if (result == null) return null;
3228

3329
if (isPromise(result)) {

‎lib/src/importer/js_to_dart/sync.dart

+3-5
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import '../../js/importer.dart';
1010
import '../../js/url.dart';
1111
import '../../js/utils.dart';
1212
import '../../util/nullable.dart';
13+
import '../canonicalize_context.dart';
1314
import 'utils.dart';
1415

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

3637
Uri? canonicalize(Uri url) {
37-
var result = wrapJSExceptions(() => _canonicalize(
38-
url.toString(),
39-
CanonicalizeContext(
40-
fromImport: fromImport,
41-
containingUrl: containingUrl.andThen(dartToJSUrl))));
38+
var result = wrapJSExceptions(
39+
() => _canonicalize(url.toString(), canonicalizeContext));
4240
if (result == null) return null;
4341
if (isJSUrl(result)) return jsToDartUrl(result as JSUrl);
4442

‎lib/src/importer/utils.dart

+22-16
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'dart:async';
77
import 'package:path/path.dart' as p;
88

99
import '../io.dart';
10+
import './canonicalize_context.dart';
1011

1112
/// Whether the Sass compiler is currently evaluating an `@import` rule.
1213
///
@@ -15,30 +16,35 @@ import '../io.dart';
1516
/// canonicalization should be identical for `@import` and `@use` rules. It's
1617
/// admittedly hacky to set this globally, but `@import` will eventually be
1718
/// removed, at which point we can delete this and have one consistent behavior.
18-
bool get fromImport => Zone.current[#_inImportRule] as bool? ?? false;
19-
20-
/// The URL of the stylesheet that contains the current load.
21-
Uri? get containingUrl => switch (Zone.current[#_containingUrl]) {
19+
bool get fromImport =>
20+
((Zone.current[#_canonicalizeContext] as CanonicalizeContext?)
21+
?.fromImport ??
22+
false);
23+
24+
/// The CanonicalizeContext of the current load.
25+
CanonicalizeContext get canonicalizeContext =>
26+
switch (Zone.current[#_canonicalizeContext]) {
2227
null => throw StateError(
23-
"containingUrl may only be accessed within a call to canonicalize()."),
24-
#_none => null,
25-
Uri url => url,
28+
"canonicalizeContext may only be accessed within a call to canonicalize()."),
29+
CanonicalizeContext context => context,
2630
var value => throw StateError(
27-
"Unexpected Zone.current[#_containingUrl] value $value.")
31+
"Unexpected Zone.current[#_canonicalizeContext] value $value.")
2832
};
2933

3034
/// Runs [callback] in a context where [fromImport] returns `true` and
3135
/// [resolveImportPath] uses `@import` semantics rather than `@use` semantics.
3236
T inImportRule<T>(T callback()) =>
33-
runZoned(callback, zoneValues: {#_inImportRule: true});
37+
switch (Zone.current[#_canonicalizeContext]) {
38+
null => runZoned(callback,
39+
zoneValues: {#_canonicalizeContext: CanonicalizeContext(null, true)}),
40+
CanonicalizeContext context => context.withFromImport(true, callback),
41+
var value => throw StateError(
42+
"Unexpected Zone.current[#_canonicalizeContext] value $value.")
43+
};
3444

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

4349
/// Resolves an imported path using the same logic as the filesystem importer.
4450
///

‎lib/src/js.dart

+2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'package:js/js_util.dart';
77
import 'js/exception.dart';
88
import 'js/deprecations.dart';
99
import 'js/exports.dart';
10+
import 'js/importer/canonicalize_context.dart';
1011
import 'js/compile.dart';
1112
import 'js/compiler.dart';
1213
import 'js/legacy.dart';
@@ -64,6 +65,7 @@ void main() {
6465
"dart2js\t${const String.fromEnvironment('dart-version')}\t"
6566
"(Dart Compiler)\t[Dart]";
6667

68+
updateCanonicalizeContextPrototype();
6769
updateSourceSpanPrototype();
6870

6971
// Legacy API

‎lib/src/js/importer.dart

+1-9
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import 'package:js/js.dart';
66

7+
import '../importer/canonicalize_context.dart';
78
import 'url.dart';
89

910
@JS()
@@ -15,15 +16,6 @@ class JSImporter {
1516
external Object? get nonCanonicalScheme;
1617
}
1718

18-
@JS()
19-
@anonymous
20-
class CanonicalizeContext {
21-
external bool get fromImport;
22-
external JSUrl? get containingUrl;
23-
24-
external factory CanonicalizeContext({bool fromImport, JSUrl? containingUrl});
25-
}
26-
2719
@JS()
2820
@anonymous
2921
class JSImporterResult {

‎lib/src/js/importer/canonicalize_context.dart

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright 2014 Google Inc. Use of this source code is governed by an
2+
// MIT-style license that can be found in the LICENSE file or at
3+
// https://opensource.org/licenses/MIT.
4+
5+
import '../../importer/canonicalize_context.dart';
6+
import '../../util/nullable.dart';
7+
import '../reflection.dart';
8+
import '../utils.dart';
9+
10+
/// Adds JS members to Dart's `CanonicalizeContext` class.
11+
void updateCanonicalizeContextPrototype() =>
12+
getJSClass(CanonicalizeContext(null, false)).defineGetters({
13+
'fromImport': (CanonicalizeContext self) => self.fromImport,
14+
'containingUrl': (CanonicalizeContext self) =>
15+
self.containingUrl.andThen(dartToJSUrl),
16+
});

0 commit comments

Comments
 (0)