Skip to content
This repository has been archived by the owner on May 18, 2023. It is now read-only.
This repository has been archived by the owner on May 18, 2023. It is now read-only.

Add included sources to the CompileSuccess message #62

Closed
irundaia opened this issue May 14, 2021 · 2 comments · Fixed by #68
Closed

Add included sources to the CompileSuccess message #62

irundaia opened this issue May 14, 2021 · 2 comments · Fixed by #68
Labels
enhancement New feature or request

Comments

@irundaia
Copy link

I'm working with an integration of the embedded sass compiler into a web framework.
This framework supports incremental compilation of assets. To be able to correctly trigger a new compilation, in case a included file has been imported, I'd need to tell the framework which files have been imported, such that the framework will also start monitoring those files.
To that end, I propose to add a includedFiles property to the CompileSuccess message.

Would this be something you would consider accepting a PR for?

P.s.
I know that this data is available in the compile result generated by dart-sass. Is there some other implementation I should consider?

@nex3 nex3 added the enhancement New feature or request label May 19, 2021
@nex3
Copy link
Contributor

nex3 commented May 19, 2021

This is probably something we can do, although it'll require also figuring out how to expose this information in the Dart API which we use to implement the compiler. Note that we'd need to expose this as includedUrls, not includedFiles, since the compiler only thinks about stylesheets in terms of their canonical URLs.

In sass/embedded-host-node#47 (comment), @Splaktar mentioned that the Angular team also wants access to this information, but I haven't gotten a clear answer on why. I'd like to understand all use-cases before designing the protocol.

@alan-agius4
Copy link

includedFiles are needed so that we/webpacks can build a full dependency graph for the compilation, so that when one of the dependencies changes it re-triggers a re-compilation.

https://github.com/webpack-contrib/sass-loader/blob/8b1ff66fd96d6a8872c40d02a4476942d08ea37f/src/index.js#L74

I think you meant includePaths and not includeUrl, a lot of downstream users depend on these and therefore without this option it would be hard to replace dart-sass without significant changes.

We’d also need addition APIs that sass-loader needs such as importer and outputStyle.

nex3 added a commit that referenced this issue Jun 10, 2021
@nex3 nex3 closed this as completed in #68 Jun 11, 2021
nex3 added a commit that referenced this issue Jun 11, 2021
nex3 added a commit to sass/dart-sass that referenced this issue Jul 14, 2021
This also exposes CompileResult.includedUrls.

Closes #1355
Partially addresses sass/embedded-protocol#62
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants