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

update flag doesn't work with absolute source paths #2199

Open
1 of 2 tasks
sleeuwen opened this issue Mar 18, 2024 · 2 comments
Open
1 of 2 tasks

update flag doesn't work with absolute source paths #2199

sleeuwen opened this issue Mar 18, 2024 · 2 comments
Labels
bug pending deprecation Follow up after a three-month deprecation period
Milestone

Comments

@sleeuwen
Copy link

sleeuwen commented Mar 18, 2024


Hi,

I've run into a problem when using the --update flag with the cli and specifying an absolute path for the source file/directory. It seems to always update the output .css file(s) even if the scss file hasn't changed. This is causing a problem in a project that I maintain which integrates the dart-sass executable in a C#/ASP.NET Core application. This project is using the dart vm version which we download directly from the GitHub release assets.

I've tried the following to come to the conclusion that it has something to do with specifying an absolute path for the source file/folder (I only tested this on Linux):

  • Create a .scss file in a folder (doesn't have to contain anything special, can just be valid css)
  • Run sass folder:folder --update, this works as expected and only updates the css when the scss has changed
  • Run sass /absolute/path/to/folder:/absolute/path/to/folder --update or sass /absolute/path/to/folder:folder --update, this will always update the output .css file
  • Run sass folder:/absolute/path/to/folder --update, this works as expected and only updates the css when the scss changes.

Interestingly, if I specify a folder as a source and run the command multiple times it also tries to (re)compile the css file that was generated the first time. This succeeds only some of the time and fails with an error other times.

I've tried it on multiple older versions and it seems to happen since version 1.67.0. In version 1.66.1 it does work correctly.

Thanks in advance!

@nex3
Copy link
Contributor

nex3 commented Mar 21, 2024

In #2077, we (correctly) fixed a bug by making the current importer for a given file unable to load absolute URLs. Now absolute URLs are only ever handled by the set of importers the user passes in explicitly. However, this means that if the user doesn't pass in any importers that can handle absolute file: URLs, they can't be loaded at all. Although this is arguably desirable in general, since it means the importers and loadPaths options are fully authoritative for which absolute URLs can be loaded, it causes several knock-on issues.

The first knock-on issue is the one reported here. In the stylesheet graph we use to determine when updates are necessary, the import cache doesn't have a global FilesystemImporter. This means that it doesn't know how to load absolute file: URLs (which absolute paths on the CLI get converted into), so they're treated as "new files" and always recompiled.

The second knock-on issue is maybe even worse: stylesheets compiled by path on the CLI can't load absolute file: URLs at all through @use or @import. Users can work around this by passing a --load-path, but these loads should work by default.

The third knock-on issue is actually much broader, and actually predates #2077. The question is: what filesystem importer do we add to the import cache to fix the previous two issues? And we don't currently have a good answer. The problem is that there's currently only one type of filesystem importer: it takes a load path as an argument and handles both absolute file: URLs and relative URLs, which it tries to resolve relative to its load path. But in cases like the ones above, the user doesn't want to add a load path. They just want to be able to load absolute URLs.

Our current standard answer to this is FilesystemImporter.cwd, which has . as its load path. But this is unsuitable, because it means that any Sass file anywhere can @use a relative URL and have it be resolved relative to the process's working directory, which is almost certainly not intended. In fact, I think exposing FilesystemImporter.cwd at all was a mistake. Instead, I think we need to deprecate that and provide a new FilesystemImporter field that only supports loading absolute file: URLs and URLs relative to the current file.

@nex3 nex3 added this to the 2.0.0 milestone Mar 21, 2024
@nex3 nex3 added the pending deprecation Follow up after a three-month deprecation period label Mar 27, 2024
@sleeuwen
Copy link
Author

sleeuwen commented Apr 5, 2024

The latest 1.74.1 release has fixed the issue for the user of our library, thanks!

I won't close the issue as there's still a task open, but from my point of view it's fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug pending deprecation Follow up after a three-month deprecation period
Projects
None yet
Development

No branches or pull requests

2 participants