sass / dart-sass

The reference implementation of Sass, written in Dart.
https://sass-lang.com/dart-sass
MIT License
3.9k stars 352 forks source link

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

Open sleeuwen opened 6 months ago

sleeuwen commented 6 months ago

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):

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 commented 5 months ago

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.

sleeuwen commented 5 months ago

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.