While implementing the Dart Sass host, I realized that CanonicalizeResponse.file is awkward to use as currently specified. The goal of the field was to allow importers (and hosts) that redirect imports to the filesystem to avoid re-implementing Sass's logic for resolving a path for partials, file extensions, and index files. For example, WebPack's custom importer could just strip off the initial ~, convert the remaining string to a path, and return a CanonicalizeResponse with file set to that path.

Three Issues

This brings up the first issue with the current protocol: it requires handlers to convert from a URL to an OS path. This is something developers often mess up, since on Unixes URLs look like they can be used as-is in place of paths.

Also, semantics of canonicalization and importing don't work out so easily. The canonicalize handler is required to ensure that any canonicalized URLs it returns can be passed back to CanonicalizeRequest and will be returned unchanged (this is currently unspecified, but I'm planning to add it to the spec). This is the second issue: the current protocol requires handlers to detect when they're being given an absolute file: URL and handle it specially.

Finally, every importer's import handler still has to duplicate the logic of reading the file from disk. This isn't too bad, but it's needless duplication for something that will be identical for every filesystem-based importer.

Potential Solutions

No Change

We could just keep the protocol as-is, and suffer the issues I outlined above. I obviously don't think this is a good solution or I wouldn't be opening this issue.

No Special Handling

We could get rid of the CanonicalizeResponse.file field and replace it with nothing. This requires all plugin authors to correctly implement path resolution themselves, but it makes the protocol itself maximally simple.

Special-Case File Responses

We could keep the existing shape of the protocol, but require the compiler to handle CanonicalizeResponse.file specially by routing all further interactions with that file to a built-in importer rather than one provided by the user. This keeps the existing protocol (which is a good fit for the Node Sass API, if we decide we want to try to match that), but it adds implementation complexity on the compiler-side. It means that the compiler can no longer think of importers as effectively stateless, since whether it has special processing for (a subset of) file: URLs depends on whether the importer previously returned a CanonicalizeResponse.file field or not.

New Importer Type

We could define an entirely new type of importer, maybe a PathImporter, that only takes a single PathImportRequest. In Dart Sass, we'd implement this as a wrapper around FilesystemImporter (the same way we do for Dart's PackageImporter). This provides both an easy way for importer implementors to farm work off to the compiler and for the compiler to model this as a single stateless importer, since it knows ahead of time that this importer supports file: URLs that refer to the filesystem. The main downside is the added protocol complexity.

dart-sass allows setting the include path via https://sass-lang.com/documentation/cli/dart-sass#load-path . How can I do that with the sass embedded protocol?

Is there a recommended way for hosts to validate their implementation ?

I could imagine that hosts could implement a basic CLI wrapper compatible with the sass-spec runner and running sass-spec with --impl dart-sass -c ./my-host-cli. But that would probably not cover all features of the protocol (custom importers and host-provided functions for instance). So this might not be sufficient.

From what I can tell in the specification, it doesn't look like it's possible to map one import statement to multiple results. With libsass, we can implement a custom importer that takes something like this:

@import "globbed/**/*";

and returns a list of imported files (i.e. sass_make_import_list).

I'm wondering if it might be possible to have a CanonicalizeResponse include an array of URLs rather than strictly only one. Otherwise, is there another way to implement this functionality?

I had a go at implementing the host side of this in Swift. It came out OK -- I found the docs really clear and well written. Here’s some feedback on the protocol.

Binary delimiter. The protobuf docs are a bit vague on exactly how to delimit streamed messages. The implementations I’ve looked at (C Java Swift) all use a Varint32. The Sass protocol docs are clear that it uses a fixed-width uint32 so I was never confused or misled, but it may be worth changing the protocol to use Varint32 to make it easier to adopt using the standard protobuf helpers.

Version. I’d find a simple ‘give me some version information’ compiler request useful for problem characterization/debugging. In particular the Swift package infrastructure isn’t sophisticated enough to be able to declare a dependency on a Dart binary so I’m reliant on my user to assemble the right pieces: being able to check (or at least log) that the given compiler is version X of implementation Y would be useful.

CompileReq.string.importer. This importer is ignored in the code -- is that a todo or is the protobuf field a leftover?

[I had some more here about FileImporters and invoking CompilerFunctions but then I found the closed issues, so, never mind!]

Protocol buffers are not self-delimiting, so if we want to communicate over a stream that's not inherently packet-based (which we do, since we plan to use stdin/stdout), we'll need a scheme for indicating which bytes belong to which message. The "Techniques" page of the protobuf documentation suggests writing the message length before the message, which seems like as good a solution as any.

https://github.com/sass/embedded-protocol/runs/4662883885?check_suite_focus=true

fatal: ambiguous argument 'HEAD^': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'

It looks like the issue is caused by shallow git fetch of depth 1, so that HEAD^ will not be resolved, therefore it is not able to detect version change and tag new release automatically.

As a result, tag for 1.0.0-beta.17 is currently missing.

For now, only the 1.0.0-beta6 release is tagged on github.

We should set up Travis to at least verify that the protobuf definitions build successfully.

I think the documentation for Value.CompilerFunction needs some improvement:

The mentioned message type "InboundRequest.FunctionCallRequest" isn't defined anywhere.
Also "InboundMessage" which might be meant here, does not have a "FunctionCallRequest" either.

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?

Tag 1.0.0-beta.15 is missing, which is used by the latest version of dart-sass-embedded.

The changelog of dart-sass-embedded says:

Support version 1.0.0-beta.14 of the Sass embedded protocol

However, in reality it is 1.0.0-beta.15 not 1.0.0-beta.14:

$ ./dart-sass-embedded --version
{
  "protocolVersion": "1.0.0-beta.15",
  "compilerVersion": "1.0.0-beta.12",
  "implementationVersion": "1.42.1",
  "implementationName": "Dart Sass",
  "id": 0
}

Since the host doesn't have access to source contents, the protocol should allow the host to request that the compiler include source contents in the outputted source-map.

The current protocol doesn't have any way of representing a failed compilation. We should add this, with at least an error message, stack trace, and source location.

The protocol needs a way for one endpoint to signal to the other that its request was invalid. I think it's generally a good idea to do something like JSON-RPC 2.0's error messages.

I'm in the process of implementing this in Go (for Hugo), and it's mostly smooth sailing, but I have stumbled upon a blocker. This may be me not understanding the spec, but I have studied it carefully.

Ref. this:

When loading a URL, the host must first try resolving that URL relative to the canonical URL of the current file

What I then get to resolve a given import is something like this:

&embeddedsass.OutboundMessage_CanonicalizeRequest{
  Id: 0,
  CompilationId: 0,
  ImporterId: 5679,
  Url: "colors",
}

I don't see how I can determine "the current file" from the above.

Do we want to have some sort of version negotiation take place when the protocol is first connected? If we think we'll ever want this, specifying it early on will mean all implementations will support it at least well enough to say "no I don't support your protocol version". On the other hand, I expect most implementation distributions will be tightly-coupled enough that the embedder will always know which version of the compiler it's shipping (at least to within a semantic version range).

@strychnide What do you think?

This part of the CanonicalizeRequest documentation asks the host to perform special handling for sass files stored on disk:

The mentioned field CanonicalizeResponse.result.file does not exist, so this feature can not be used.

sass_embedded 1.0.0-beta.8 has been released, which targets version 1.0.0-beta.11 of this protocol, but there is no git tag or GitHub Release for version 1.0.0-beta.11

uint32 vs int32 -- I suspect these should be the same type:

The protocol currently defines 4 output styles:

dart-sass only support expanded and compressed output. This means that dart-sass-embedded will replace COMPACT and NESTED with an EXPANDED output in the response. Given that dart-sass-embedded is the only implementation of the compiler side of this protocol (and it does not make sense to implement the compiler side of this protocol on top of libsass given it is deprecated), what is the reason to support COMPACT and NESTED in the protocol ?

If some host implementation wants to expose an API reproducing the API of an existing libsass wrapper for their language (to ease the migration), they could implement this conversion of nested and compact to expanded in their own API, before converting to the embedded protocol values.

The compiler should be able to send events to the embedder for log messages, including user-generated messages from @debug and @warn and implementation-generated warnings. These should include optional metadata indicating the source of the warning and the stack trace of how it was reached.

The protocol currently allows the host to invoke first-class functions using InboundMessage.FunctionCallRequest. However, neither the Dart nor the JavaScript APIs currently support invoking first-class functions from custom functions, and there doesn't seem to be much user demand for doing so. It may be wise to just drop them from the protocol until we hear about a clear use case.

The protocol currently mandates that outbound function calls pass variable argument lists "as Value.ArgumentLists", but it doesn't actually define a Value.ArgumentList type. We should do so.

Argument lists pose a particular challenge in that they care about whether their associated keyword arguments were used in practice or not. The compiler throws an error if keyword arguments were passed but not used, since function signatures don't distinguish between taking variable length positional versus keyword arguments, and we want users to have feedback when passing keyword arguments to a function that doesn't accept them.

I think we want to handle this by always passing keyword arguments for each argument list, and adding a repeated int keywords_accessed field to InboundMessage.FunctionCallResponse that indicates which arguments' keywords were accessed in practice. Then the compiler can use this to generate errors where appropriate.

https://github.com/sass/embedded-protocol#versioning describes versioning rules regarding semver. For now, it mostly says that adding a new InboundMessage is considered to be backward compatible (I assume that this also allows to add the correspond Response message in OutboundMessage, but this is not explicitly said).

Given that semver impact for proto is not very common (at least AFAIK), it might be great to be document what exactly is allowed in a non-breaking release:

• can I add a new field in an existing OutboundMessage type ?
• can I add a new field in an existing InboundMessage type ?
• can I add a new event ? (probably no, as that's an OutboundMessage)

The embedder should be able to define functions, which can be called during compilation by outbound requests. This will also involve defining a serialization format for all Sass values.

One big question here is how we want to serialize maps and lists. We could include the full contents no matter what, but that runs the risk of creating very large messages when very large lists and maps are sent back and forth. I believe @chriseppstein has mentioned this issue in the past when dealing with embedding LibSass, so maybe he can chime in with examples of the scale of data structure involved.

The embedder should be able to define importers, which can be called during compilation by outbound requests. It should also be able to provide load paths intermixed with the custom importers.