Skip to content

Avoid relying on buggy import order in the legacy importer#245

Merged
nex3 merged 3 commits intomainfrom
importer-bug
Sep 8, 2023
Merged

Avoid relying on buggy import order in the legacy importer#245
nex3 merged 3 commits intomainfrom
importer-bug

Conversation

@nex3
Copy link
Contributor

@nex3 nex3 commented Sep 7, 2023

Historically, the legacy importer shim has strongly assumed that all
calls to canonicalize() would doubled: once as a URL resolved
relative to the current importer, and again as a URL passed to the
importers list. However, this relies on buggy, non-spec-compliant
behavior in Dart Sass: absolute URLs should never be passed to the
current importer, only to the global list of importers.

This change avoids relying on that behavior by instead disambiguating
relative loads using a custom URL protocol prefix. All canonical URLs
passed to Dart Sass now have the prefix legacy-importer- added to
the beginning. This allows us to disambiguate relative loads, and
ignore them for non-file: URLs, since relative loads and only
relative loads will have schemes beginning with legacy-importer-.

To avoid regressing the developer experience, we then strip these
prefixes from all URLs before they're passed to any developer-facing
APIs or displayed to the user.

See sass/sass-spec#1935
See sass/dart-sass#2077

Historically, the legacy importer shim has strongly assumed that all
calls to `canonicalize()` would doubled: once as a URL resolved
relative to the current importer, and again as a URL passed to the
importers list. However, this relies on buggy, non-spec-compliant
behavior in Dart Sass: absolute URLs should never be passed to the
current importer, only to the global list of importers.

This change avoids relying on that behavior by instead disambiguating
relative loads using a custom URL protocol prefix. All canonical URLs
passed to Dart Sass now have the prefix `legacy-importer-` added to
the beginning. This allows us to disambiguate relative loads, and
ignore them for non-`file:` URLs, since relative loads and _only_
relative loads will have schemes beginning with `legacy-importer-`.

To avoid regressing the developer experience, we then strip these
prefixes from all URLs before they're passed to any developer-facing
APIs or displayed to the user.
@nex3 nex3 merged commit 6129adf into main Sep 8, 2023
@nex3 nex3 deleted the importer-bug branch September 8, 2023 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants