Implement the host side of the deprecations API #279
Conversation
lib/src/deprecations.ts
Outdated
There was a problem hiding this comment.
I think it would be better to not duplicate this list of deprecations on every host implementation. Either that we just don't expose the list of all deprecations as part of the JS API, or we implement a protocol to allow embedded host to send query to compiler and ask this information.
There was a problem hiding this comment.
Not having any metadata as part of the client API would mean that TypeScript users don't get any static checking of their deprecation names. That's not an unbearable cost, but I'd prefer to have better devx there if possible.
I do agree that we need a better story for synchronizing the deprecation list across various repos and packages, though. As it stands, every time we need to add a new deprecation it'll have to be added to Dart Sass, the language repo, and the embedded host separately even if it's a pure language-level deprecation that has no bearing on the JS API. What's worse, I don't think we have any verification in place that will fail if we forget one or more of these locations.
I wonder if it makes sense to include a JSON file (or a text proto?) that just lists all the known deprecations, from which we can generate the code representations. Then we can have simple diff tests that verify that all the generated versions are based on the latest deprecation list. @jathak what do you think?
There was a problem hiding this comment.
I like the idea of maintaining a text proto, which can generate ts definition and be directly used as argument in embedded-protocol.
There was a problem hiding this comment.
Yeah, I think having a single representation and generating code for each implementation makes sense. I'd vote for writing it in YAML since it's a better authoring experience than JSON and I don't think the Dart protobuf library supports parsing text protos (google/protobuf.dart#125).
Should this block releasing the deprecations API, or should we release it with just a simple synchronization test and then switch to generating the code afterwards?
| @@ -137,6 +141,12 @@ export function newCompileStringRequest( | |||
| return request; | |||
| } | |||
|
|
|||
| function validDeprecationId( | |||
lib/src/deprecations.ts
Outdated
There was a problem hiding this comment.
Not having any metadata as part of the client API would mean that TypeScript users don't get any static checking of their deprecation names. That's not an unbearable cost, but I'd prefer to have better devx there if possible.
I do agree that we need a better story for synchronizing the deprecation list across various repos and packages, though. As it stands, every time we need to add a new deprecation it'll have to be added to Dart Sass, the language repo, and the embedded host separately even if it's a pure language-level deprecation that has no bearing on the JS API. What's worse, I don't think we have any verification in place that will fail if we forget one or more of these locations.
I wonder if it makes sense to include a JSON file (or a text proto?) that just lists all the known deprecations, from which we can generate the code representations. Then we can have simple diff tests that verify that all the generated versions are based on the latest deprecation list. @jathak what do you think?
package.json
Outdated
| "version": "1.72.0", | ||
| "protocol-version": "2.5.0", | ||
| "compiler-version": "1.72.0", | ||
| "version": "1.74.0", |
There was a problem hiding this comment.
You don't need to update the version and compiler-version here manually, it's done as part of the release process.
There was a problem hiding this comment.
Ah. I got the error for the protocol version and just updated all of them. Fixed.
See sass/sass#3826
See sass/sass-spec#1967
See sass/dart-sass#2207