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 1 commit
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
2 changes: 2 additions & 0 deletions lib/src/embedded/importer/file.dart
Expand Up @@ -21,10 +21,12 @@ final class FileImporter extends ImporterBase {
..importerId = _importerId
..url = url.toString()
..fromImport = fromImport;
var containingUrl = canonicalizeContext.containingUrlWithoutMarking;
if (containingUrl case var containingUrl?) {
ntkme marked this conversation as resolved.
Show resolved Hide resolved
request.containingUrl = containingUrl.toString();
}
var response = dispatcher.sendFileImportRequest(request);
if (!response.containingUrlUnused) canonicalizeContext.containingUrl;

switch (response.whichResult()) {
case InboundMessage_FileImportResponse_Result.fileUrl:
Expand Down
2 changes: 2 additions & 0 deletions lib/src/embedded/importer/host.dart
Expand Up @@ -35,10 +35,12 @@ final class HostImporter extends ImporterBase {
..importerId = _importerId
..url = url.toString()
..fromImport = fromImport;
var containingUrl = canonicalizeContext.containingUrlWithoutMarking;
if (containingUrl 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
49 changes: 49 additions & 0 deletions lib/src/importer/canonicalize_context.dart
@@ -0,0 +1,49 @@
// Copyright 2014 Google Inc. Use of this source code is governed by an
ntkme marked this conversation as resolved.
Show resolved Hide resolved
// MIT-style license that can be found in the LICENSE file or at
// https://opensource.org/licenses/MIT.

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.
///
/// @nodoc
@internal
ntkme marked this conversation as resolved.
Show resolved Hide resolved
bool get wasContainingUrlAccessed => _wasContainingUrlAccessed;
var _wasContainingUrlAccessed = false;

/// Runs [callback] in a context with specificed [fromImport].
///
/// @nodoc
@internal
T withFromImport<T>(bool fromImport, T callback()) {
nex3 marked this conversation as resolved.
Show resolved Hide resolved
var oldFromImport = _fromImport;
_fromImport = fromImport;
try {
return callback();
} finally {
_fromImport = oldFromImport;
}
}

CanonicalizeContext(this._containingUrl, this._fromImport);
}
7 changes: 2 additions & 5 deletions lib/src/importer/js_to_dart/async.dart
Expand Up @@ -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;
Expand All @@ -39,10 +39,7 @@ final class JSToDartAsyncImporter extends AsyncImporter {

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

Expand Down
8 changes: 2 additions & 6 deletions lib/src/importer/js_to_dart/async_file.dart
Expand Up @@ -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';
Expand All @@ -21,18 +20,15 @@ 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);

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))));
url.toString(), dartToJSCanonicalizeContext(canonicalizeContext)));
if (isPromise(result)) result = await promiseToFuture(result as Promise);
if (result == null) return null;
if (!isJSUrl(result)) {
Expand Down
8 changes: 2 additions & 6 deletions lib/src/importer/js_to_dart/file.dart
Expand Up @@ -9,25 +9,21 @@ 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);

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))));
url.toString(), dartToJSCanonicalizeContext(canonicalizeContext)));
if (result == null) return null;

if (isPromise(result)) {
Expand Down
7 changes: 2 additions & 5 deletions lib/src/importer/js_to_dart/sync.dart
Expand Up @@ -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;
Expand All @@ -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(), dartToJSCanonicalizeContext(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
8 changes: 3 additions & 5 deletions lib/src/js/importer.dart
Expand Up @@ -9,19 +9,17 @@ import 'url.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;

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

@JS()
Expand Down
21 changes: 21 additions & 0 deletions lib/src/js/importer/canonicalize_context.dart
@@ -0,0 +1,21 @@
// 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:js/js.dart';

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

@JSExport()
class JSExportCanonicalizeContext {
final CanonicalizeContext _canonicalizeContext;

bool get fromImport => _canonicalizeContext.fromImport;
JSUrl? get containingUrl =>
_canonicalizeContext.containingUrl.andThen(dartToJSUrl);

JSExportCanonicalizeContext(this._canonicalizeContext);
}