Make loaded_files available for failed compilations

[nex3]

For hosts implementing filesystem watcher APIs, it's important to know which files to watch when determining when to retry a compilation. The CompileSuccess.loaded_urls field exposes this information, but it's not available for failed compilations which leaves hosts unable to determine what to watch if the initial compilation fails. We should move it to CompileResult.loaded_urls (although for backwards compatibility we should leave CompileSuccess.loaded_urls as well).

[OlaviSau]

Adding context here, since this seems to be the correct place / upcoming discussion.

nex3:

I think the best solution here would probably be to move loaded_urls from CompileSuccess to CompileResult, so it's available even when compilation fails. But in the interim, you can also define a custom loader that acts as a filesystem loader while also recording all the files that get touched in the compilation process. See the spec for details on how filesystem loading should work.

my response:

@nex3 I like both of your ideas, but I actually tried the custom loader and found that it only uses the loader when it can't find the file using normal means, thus it does not produce the full file list. If that was changed and it would be possible to provide files manually - it could offer other benefits as well.

The reason being is that watch without control over loading is inefficient; on each compile - all the files are loaded again on each compile, even though most changes only involve a single file. I am not sure how much this really affects performance with modern SSD's but it's definitely a possible improvement area.

[OlaviSau]

I'll add a bit more info - I tried both Importer and FileImporter and I even debugged a bit on the sass host and it does not even receive a message about files that can be resolved without assistance. The intent seems to be that the importer is only for non standard imports.

This behavior seems to match the spec as well, notice how importers trigger after the normal resolution did not return null. https://github.com/sass/sass/blob/main/spec/modules.md#loading-a-source-file

[ntkme]

We should move it to CompileResult.loaded_urls (although for backwards compatibility we should leave CompileSuccess.loaded_urls as well).

As this protocol is internal between host and compiler and not visible to end users, making breaking changes only impacts a very small group of developers and does not impact end users. Therefore, I'd rather just take the breaking change and go to protocol 2.0 if necessary.

[nex3]

I like both of your ideas, but I actually tried the custom loader and found that it only uses the loader when it can't find the file using normal means, thus it does not produce the full file list. If that was changed and it would be possible to provide files manually - it could offer other benefits as well.

The key point here is "normal means"—if you run all imports exclusively through a custom importer, without using a filesystem importer or any load paths, the importer will have full control and visibility into all loads. (Although the cost of this is that you then need to manually reimplement all the filesystem loading logic.)

As this protocol is internal between host and compiler and not visible to end users, making breaking changes only impacts a very small group of developers and does not impact end users. Therefore, I'd rather just take the breaking change and go to protocol 2.0 if necessary.

All right, then let's make this 2.0 once it releases.