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

Expose the containing URL to importers under some circumstances #2083

Merged
merged 1 commit into from
Sep 18, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ jobs:
sass_spec_js_embedded:
name: 'JS API Tests | Embedded | Node ${{ matrix.node-version }} | ${{ matrix.os }}'
runs-on: ${{ matrix.os }}-latest
if: "github.event_name != 'pull_request' || !contains(github.event.pull_request.body, 'skip sass-embedded')"

strategy:
fail-fast: false
Expand Down
76 changes: 47 additions & 29 deletions lib/src/async_import_cache.dart
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ final class AsyncImportCache {

/// Canonicalizes [url] according to one of this cache's importers.
///
/// The [baseUrl] should be the canonical URL of the stylesheet that contains
/// the load, if it exists.
///
/// Returns the importer that was used to canonicalize [url], the canonical
/// URL, and the URL that was passed to the importer (which may be resolved
/// relative to [baseUrl] if it's passed).
Expand All @@ -139,33 +142,30 @@ final class AsyncImportCache {
if (isBrowser &&
(baseImporter == null || baseImporter is NoOpImporter) &&
_importers.isEmpty) {
throw "Custom importers are required to load stylesheets when compiling in the browser.";
throw "Custom importers are required to load stylesheets when compiling "
"in the browser.";
}

if (baseImporter != null && url.scheme == '') {
var relativeResult = await putIfAbsentAsync(_relativeCanonicalizeCache, (
url,
forImport: forImport,
baseImporter: baseImporter,
baseUrl: baseUrl
), () async {
var resolvedUrl = baseUrl?.resolveUri(url) ?? url;
if (await _canonicalize(baseImporter, resolvedUrl, forImport)
case var canonicalUrl?) {
return (baseImporter, canonicalUrl, originalUrl: resolvedUrl);
} else {
return null;
}
});
var relativeResult = await putIfAbsentAsync(
_relativeCanonicalizeCache,
(
url,
forImport: forImport,
baseImporter: baseImporter,
baseUrl: baseUrl
),
() => _canonicalize(baseImporter, baseUrl?.resolveUri(url) ?? url,
baseUrl, forImport));
if (relativeResult != null) return relativeResult;
}

return await putIfAbsentAsync(
_canonicalizeCache, (url, forImport: forImport), () async {
for (var importer in _importers) {
if (await _canonicalize(importer, url, forImport)
case var canonicalUrl?) {
return (importer, canonicalUrl, originalUrl: url);
if (await _canonicalize(importer, url, baseUrl, forImport)
case var result?) {
return result;
}
}

Expand All @@ -175,18 +175,36 @@ final class AsyncImportCache {

/// Calls [importer.canonicalize] and prints a deprecation warning if it
/// returns a relative URL.
Future<Uri?> _canonicalize(
AsyncImporter importer, Uri url, bool forImport) async {
var result = await (forImport
? inImportRule(() => importer.canonicalize(url))
: importer.canonicalize(url));
if (result?.scheme == '') {
_logger.warnForDeprecation(Deprecation.relativeCanonical, """
Importer $importer canonicalized $url to $result.
Relative canonical URLs are deprecated and will eventually be disallowed.
""");
///
/// If [resolveUrl] is `true`, this resolves [url] relative to [baseUrl]
/// before passing it to [importer].
Future<AsyncCanonicalizeResult?> _canonicalize(
AsyncImporter importer, Uri url, Uri? baseUrl, bool forImport,
{bool resolveUrl = false}) async {
var resolved =
resolveUrl && baseUrl != null ? baseUrl.resolveUri(url) : url;
var canonicalize = forImport
? () => inImportRule(() => importer.canonicalize(resolved))
: () => importer.canonicalize(resolved);

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

if (result.scheme == '') {
_logger.warnForDeprecation(
Deprecation.relativeCanonical,
"Importer $importer canonicalized $resolved to $result.\n"
"Relative canonical URLs are deprecated and will eventually be "
"disallowed.");
} else if (await importer.isNonCanonicalScheme(result.scheme)) {
throw "Importer $importer canonicalized $resolved to $result, which "
"uses a scheme declared as non-canonical.";
}
return result;

return (importer, result, originalUrl: resolved);
}

/// Tries to import [url] using one of this cache's importers.
Expand Down
19 changes: 15 additions & 4 deletions lib/src/embedded/dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -206,19 +206,32 @@ final class Dispatcher {
InboundMessage_CompileRequest_Importer importer) {
switch (importer.whichImporter()) {
case InboundMessage_CompileRequest_Importer_Importer.path:
_checkNoNonCanonicalScheme(importer);
return sass.FilesystemImporter(importer.path);

case InboundMessage_CompileRequest_Importer_Importer.importerId:
return HostImporter(this, importer.importerId);
return HostImporter(
this, importer.importerId, importer.nonCanonicalScheme);

case InboundMessage_CompileRequest_Importer_Importer.fileImporterId:
_checkNoNonCanonicalScheme(importer);
return FileImporter(this, importer.fileImporterId);

case InboundMessage_CompileRequest_Importer_Importer.notSet:
_checkNoNonCanonicalScheme(importer);
return null;
}
}

/// Throws a [ProtocolError] if [importer] contains one or more
/// `nonCanonicalScheme`s.
void _checkNoNonCanonicalScheme(
InboundMessage_CompileRequest_Importer importer) {
if (importer.nonCanonicalScheme.isEmpty) return;
throw paramsError("Importer.non_canonical_scheme may only be set along "
"with Importer.importer.importer_id");
}

/// Sends [event] to the host.
void sendLog(OutboundMessage_LogEvent event) =>
_send(OutboundMessage()..logEvent = event);
Expand Down Expand Up @@ -278,9 +291,7 @@ final class Dispatcher {
InboundMessage_Message.versionRequest =>
throw paramsError("VersionRequest must have compilation ID 0."),
InboundMessage_Message.notSet =>
throw parseError("InboundMessage.message is not set."),
_ =>
throw parseError("Unknown message type: ${message.toDebugString()}")
throw parseError("InboundMessage.message is not set.")
};

if (message.id != _outboundRequestId) {
Expand Down
15 changes: 10 additions & 5 deletions lib/src/embedded/importer/file.dart
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,14 @@ final class FileImporter extends ImporterBase {
Uri? canonicalize(Uri url) {
if (url.scheme == 'file') return _filesystemImporter.canonicalize(url);

var response =
dispatcher.sendFileImportRequest(OutboundMessage_FileImportRequest()
..importerId = _importerId
..url = url.toString()
..fromImport = fromImport);
var request = OutboundMessage_FileImportRequest()
..importerId = _importerId
..url = url.toString()
..fromImport = fromImport;
if (containingUrl case var containingUrl?) {
request.containingUrl = containingUrl.toString();
}
var response = dispatcher.sendFileImportRequest(request);

switch (response.whichResult()) {
case InboundMessage_FileImportResponse_Result.fileUrl:
Expand All @@ -49,5 +52,7 @@ final class FileImporter extends ImporterBase {

ImporterResult? load(Uri url) => _filesystemImporter.load(url);

bool isNonCanonicalScheme(String scheme) => scheme != 'file';

String toString() => "FileImporter";
}
35 changes: 29 additions & 6 deletions lib/src/embedded/importer/host.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
// MIT-style license that can be found in the LICENSE file or at
// https://opensource.org/licenses/MIT.

import '../../exception.dart';
import '../../importer.dart';
import '../../importer/utils.dart';
import '../../util/span.dart';
import '../dispatcher.dart';
import '../embedded_sass.pb.dart' hide SourceSpan;
import '../utils.dart';
Expand All @@ -13,14 +16,31 @@ final class HostImporter extends ImporterBase {
/// The host-provided ID of the importer to invoke.
final int _importerId;

HostImporter(Dispatcher dispatcher, this._importerId) : super(dispatcher);
/// The set of URL schemes that this importer promises never to return from
/// [canonicalize].
final Set<String> _nonCanonicalSchemes;

HostImporter(Dispatcher dispatcher, this._importerId,
Iterable<String> nonCanonicalSchemes)
: _nonCanonicalSchemes = Set.unmodifiable(nonCanonicalSchemes),
super(dispatcher) {
for (var scheme in _nonCanonicalSchemes) {
if (isValidUrlScheme(scheme)) continue;
throw SassException(
'"$scheme" isn\'t a valid URL scheme (for example "file").',
bogusSpan);
}
}

Uri? canonicalize(Uri url) {
var response =
dispatcher.sendCanonicalizeRequest(OutboundMessage_CanonicalizeRequest()
..importerId = _importerId
..url = url.toString()
..fromImport = fromImport);
var request = OutboundMessage_CanonicalizeRequest()
..importerId = _importerId
..url = url.toString()
..fromImport = fromImport;
if (containingUrl case var containingUrl?) {
request.containingUrl = containingUrl.toString();
}
var response = dispatcher.sendCanonicalizeRequest(request);

return switch (response.whichResult()) {
InboundMessage_CanonicalizeResponse_Result.url =>
Expand All @@ -47,5 +67,8 @@ final class HostImporter extends ImporterBase {
};
}

bool isNonCanonicalScheme(String scheme) =>
_nonCanonicalSchemes.contains(scheme);

String toString() => "HostImporter";
}
74 changes: 46 additions & 28 deletions lib/src/import_cache.dart
Original file line number Diff line number Diff line change
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: 3e4cae79c03ce2af6626b1822f1468523b401e90
// Checksum: ff52307a3bc93358ddc46f1e76120894fa3e071f
//
// ignore_for_file: unused_import

Expand Down Expand Up @@ -124,6 +124,9 @@ final class ImportCache {

/// Canonicalizes [url] according to one of this cache's importers.
///
/// The [baseUrl] should be the canonical URL of the stylesheet that contains
/// the load, if it exists.
///
/// Returns the importer that was used to canonicalize [url], the canonical
/// URL, and the URL that was passed to the importer (which may be resolved
/// relative to [baseUrl] if it's passed).
Expand All @@ -139,31 +142,27 @@ final class ImportCache {
if (isBrowser &&
(baseImporter == null || baseImporter is NoOpImporter) &&
_importers.isEmpty) {
throw "Custom importers are required to load stylesheets when compiling in the browser.";
throw "Custom importers are required to load stylesheets when compiling "
"in the browser.";
}

if (baseImporter != null && url.scheme == '') {
var relativeResult = _relativeCanonicalizeCache.putIfAbsent((
url,
forImport: forImport,
baseImporter: baseImporter,
baseUrl: baseUrl
), () {
var resolvedUrl = baseUrl?.resolveUri(url) ?? url;
if (_canonicalize(baseImporter, resolvedUrl, forImport)
case var canonicalUrl?) {
return (baseImporter, canonicalUrl, originalUrl: resolvedUrl);
} else {
return null;
}
});
var relativeResult = _relativeCanonicalizeCache.putIfAbsent(
(
url,
forImport: forImport,
baseImporter: baseImporter,
baseUrl: baseUrl
),
() => _canonicalize(baseImporter, baseUrl?.resolveUri(url) ?? url,
baseUrl, forImport));
if (relativeResult != null) return relativeResult;
}

return _canonicalizeCache.putIfAbsent((url, forImport: forImport), () {
for (var importer in _importers) {
if (_canonicalize(importer, url, forImport) case var canonicalUrl?) {
return (importer, canonicalUrl, originalUrl: url);
if (_canonicalize(importer, url, baseUrl, forImport) case var result?) {
return result;
}
}

Expand All @@ -173,17 +172,36 @@ final class ImportCache {

/// Calls [importer.canonicalize] and prints a deprecation warning if it
/// returns a relative URL.
Uri? _canonicalize(Importer importer, Uri url, bool forImport) {
var result = (forImport
? inImportRule(() => importer.canonicalize(url))
: importer.canonicalize(url));
if (result?.scheme == '') {
_logger.warnForDeprecation(Deprecation.relativeCanonical, """
Importer $importer canonicalized $url to $result.
Relative canonical URLs are deprecated and will eventually be disallowed.
""");
///
/// If [resolveUrl] is `true`, this resolves [url] relative to [baseUrl]
/// before passing it to [importer].
CanonicalizeResult? _canonicalize(
Importer importer, Uri url, Uri? baseUrl, bool forImport,
{bool resolveUrl = false}) {
var resolved =
resolveUrl && baseUrl != null ? baseUrl.resolveUri(url) : url;
var canonicalize = forImport
? () => inImportRule(() => importer.canonicalize(resolved))
: () => importer.canonicalize(resolved);

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

if (result.scheme == '') {
_logger.warnForDeprecation(
Deprecation.relativeCanonical,
"Importer $importer canonicalized $resolved to $result.\n"
"Relative canonical URLs are deprecated and will eventually be "
"disallowed.");
} else if (importer.isNonCanonicalScheme(result.scheme)) {
throw "Importer $importer canonicalized $resolved to $result, which "
"uses a scheme declared as non-canonical.";
}
return result;

return (importer, result, originalUrl: resolved);
}

/// Tries to import [url] using one of this cache's importers.
Expand Down
2 changes: 2 additions & 0 deletions lib/src/importer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,6 @@ abstract class Importer extends AsyncImporter {
DateTime modificationTime(Uri url) => DateTime.now();

bool couldCanonicalize(Uri url, Uri canonicalUrl) => true;

bool isNonCanonicalScheme(String scheme) => false;
}
Loading