Skip to content

fix: emit a file URL for the styles configFile - #397

Open
haigou-web wants to merge 3 commits into
vuetifyjs:mainfrom
haigou-web:fix/windows-settings-scss-url
Open

haigou-web wants to merge 3 commits into
vuetifyjs:mainfrom
haigou-web:fix/windows-settings-scss-url

Conversation

@haigou-web

Copy link
Copy Markdown

Fixes #396.

Root cause

Not spaces — the resolved path is passed to @use as a bare Windows path.

getTemplate interpolates ctx.stylesConfigFile (the result of resolvePath) straight into @use '...'. Dart Sass resolves @use arguments as URLs, so C:/foo/settings.scss is parsed as the scheme c: and never reaches the filesystem importer:

Error: Can't find stylesheet to import.
  ╷
1 │ @use 'C:/pirepro/real/proj-nospace/styles/settings.scss';
  │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

On POSIX the resolved path starts with /, which Sass accepts as an absolute filesystem path, so this only breaks on Windows — which matches the "does it work on a non-Windows OS?" question in the issue. Spaces are incidental: the reporter's path happened to contain one, but a path without spaces fails identically.

Fix

Emit a file:// URL for absolute paths. Sass resolves those on every platform, and pathToFileURL percent-encodes spaces, so both cases work. Relative/package specifiers (vuetify/styles) are left untouched.

Verification

With sass@1.105.1, against a real settings file that has no vuetify import:

@use argument result
'C:/.../proj with space/styles/settings.scss' ❌ Can't find stylesheet
'C:/.../proj-nospace/styles/settings.scss' ❌ Can't find stylesheet
'file:///C:/.../proj%20with%20space/styles/settings.scss' ✅ ok
'file:///C:/.../proj-nospace/styles/settings.scss' ✅ ok

Note the before/after error shapes: with the bare path the failing line is the @use itself (the file was never opened), whereas with the file URL the file is opened — the failure moves inside it.

Tests

vitest run --exclude 'test/e2e/**' → 95 passed, 1 skipped.

There is one failing test, test/basic-vuetify3.test.ts → "does not inline the Vuetify 4 cascade-layer order (#381)". It is pre-existing: I stashed this change and the same test fails on main (verified). Unrelated to this patch.

Dart Sass resolves `@use` arguments as URLs, so a bare Windows path such as
`C:/foo/settings.scss` is parsed as the scheme `c:` and never reaches the
filesystem importer, failing with "Can't find stylesheet to import". POSIX
paths start with `/`, which Sass accepts, so this only breaks on Windows and
is unrelated to spaces in the path.

Emit `file://` URLs for absolute paths instead, which Sass resolves on every
platform and which also handle spaces correctly.

Verified with sass 1.105.1:
  @use 'C:/.../proj with space/styles/settings.scss'  -> Can't find stylesheet
  @use 'C:/.../proj-nospace/styles/settings.scss'     -> Can't find stylesheet
  @use 'file:///C:/.../proj%20with%20space/...'       -> ok
  @use 'file:///C:/.../proj-nospace/...'              -> ok

Refs vuetifyjs#396
…he helper

Review follow-up to 72993a1.

`pathToFileURL` leaves `'` alone -- RFC 3986 lists it as a sub-delimiter, so it
is legal in a URL path -- and `getTemplate` wraps the result in SINGLE quotes.
A settings file under `O'Brien/` therefore produced

    @use 'file:///.../O'Brien/settings.scss';

which ends the string early: a stylesheet that is a syntax error rather than a
stylesheet. A bare path had the same hole, so this closes one that was already
open rather than one this change opened.

`toSassImportUrl` is now exported and covered by a test file. It is a pure
string transform, and the Windows branch it exists for cannot be reached at all
on a non-Windows CI, so a test is the only thing between a refactor and a silent
regression. The assertions are about "is this a `file:` URL", not about one
exact encoding, because `pathToFileURL` follows the host platform.

Also documented: POSIX absolute paths are URL-ised too (the same vocabulary
upstream `@vuetify/unplugin-styles` uses for both its `sassPath` and its
`configFile`), a relative path comes back unchanged, and a UNC path becomes a
`file://host/...` URL whose non-empty host Dart Sass's filesystem importer may
refuse to map -- unreproduced here, so noted rather than guessed at.

Refs vuetifyjs#396

Signed-off-by: haigou-web <haigou-web@users.noreply.github.com>
@haigou-web

Copy link
Copy Markdown
Author

Pushed e21b48e, a review follow-up to 72993a1.

pathToFileURL leaves ' alone — RFC 3986 lists it as a sub-delimiter, so it is legal in a URL path — and getTemplate wraps the result in single quotes. A settings file under O'Brien/ therefore produced

@use 'file:///.../O'Brien/settings.scss';

which ends the string early: a stylesheet that is a syntax error rather than a stylesheet. A bare path had the same hole, so this closes one that was already open rather than one this change introduced.

toSassImportUrl is now exported and covered by a test file. It is a pure string transform, and the Windows branch it exists for cannot be reached at all on a non-Windows CI, so a test is the only thing standing between a refactor and a silent regression. The assertions are about "is this a file: URL", not one exact encoding, because pathToFileURL follows the host platform.

Also documented in the helper: POSIX absolute paths are URL-ised too (the same vocabulary upstream @vuetify/unplugin-styles uses for both sassPath and configFile), a relative path comes back unchanged, and a UNC path becomes a file://host/... URL whose non-empty host Dart Sass's filesystem importer may refuse to map — noted rather than guessed at, since it is unreproduced here.

… both branches

node:path.isAbsolute follows the host platform, so a Windows drive path was
treated as relative on POSIX — which made the new unit test fail there and left
the Windows branch untested. pathe.isAbsolute is already the module's path
vocabulary elsewhere. The apostrophe encoding no longer depends on the path
being absolute, since this is an exported pure transform.

This branch has not been deployed

No deployments
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.

Spaces in path to configFile break build

1 participant