Integrate the deprecations API into the spec/docs/protocol by jathak · Pull Request #3826 · sass/sass

jathak · 2024-04-01T18:34:14Z

In the process of implementing this, I made two minor changes to the embedded protocol part of the proposal: (1) updating the field IDs since a new field got added to CompileRequest since the proposal was written and (2) adding a deprecation_type field to LogEvent so that the embedded host can pass that information to the JS API. Let me know if these changes are fine to just do here, or if I should factor them out into a separate PR.

[skip dart-sass]
[skip sass-embedded]

nex3 · 2024-04-02T06:43:31Z

js-api-doc/deprecations.d.ts

+  /**
+   * Used for any user-emitted deprecation warnings.
+   */
+  'user-authored': Deprecation<'user-authored', 'user'>;


Do we have any testing in place that will catch if we add a deprecation to Dart Sass's enum that isn't listed here? If not, we should—I'm confident I'll make that mistake in the future 😅

I've added a test to sass/sass-spec#1967 to ensure that Dart Sass and the embedded host don't expose any extra deprecations, which I think should be sufficient until we switch the deprecations list to a single source-of-truth (tracked in #3827)

nex3 · 2024-04-02T06:44:28Z

js-api-doc/logger/index.d.ts

   *
   * @param message - The warning message.
   * @param options.deprecation - Whether this is a deprecation warning.
+   * @param options.dperecationType - The type of deprecation this warning is


Suggested change

* @param options.dperecationType - The type of deprecation this warning is

* @param options.deprecationType - The type of deprecation this warning is

nex3 · 2024-04-02T06:45:27Z

js-api-doc/options.d.ts

+   * compilation, the compiler will ignore it instead.
+   *
+   * **Heads up!** The deprecated functionality you're depending on will
+   * eventually break


Nit: period

nex3 · 2024-04-02T06:45:51Z

js-api-doc/deprecations.d.ts

+  readonly patch: number;
+
+  /**
+   * Parses a version from a string.


This should probably document that it'll throw an error if the version can't be parsed.

nex3 · 2024-04-02T06:46:53Z

spec/js-api/deprecations.d.ts.md

+> Interfaces for user-declared importers that customize how Sass loads
+> stylesheet dependencies.


Copy-paste error

nex3 · 2024-04-02T06:47:35Z

spec/js-api/deprecations.d.ts.md

+export interface Deprecations {
+```
+
+#### `call-string`


I think we can get away without summaries here, since the individual specs say "emit a deprecation named ...".

You could even get rid of the headers and have this all be a single code block.

nex3 · 2024-04-02T06:49:05Z

spec/js-api/deprecations.d.ts.md

+The compiler version this feature was fully removed in, making the deprecation
+obsolete.


Probably worth mentioning that this is also implementation-dependent.

nex3 · 2024-04-02T06:50:00Z

spec/js-api/deprecations.d.ts.md

+
+The major version.
+
+This must be a non-negative integer.


Since the spec is describing how an implementation should behave, it should define what happens if the number is negative rather than just declaring that it must not be.

Done (updated the constructor spec to say the compiler should throw an error)

spec/js-api/deprecations.d.ts.md

nex3 · 2024-04-02T20:40:13Z

We should also explicitly specify what happens if the host refers to a deprecation ID that the compiler doesn't recognize or vice-versa. In the former case, I think the compiler should just throw an error, because the host should generally be responsible for ensuring it's speaking a compatible protocol to the compiler. In the latter case, I think it's fine to say that the warning is treated as a non-deprecation.

nex3 · 2024-04-02T20:41:35Z

js-api-doc/logger/index.d.ts

    message: string,
    options: {
      deprecation: boolean;
+      deprecationType?: Deprecation;


We should probably type this explicitly so that deprecationType is set if and only if deprecation is true.

Done. Is this the cleanest way to write this type, or is there some fancy Typescript feature to make deprecationType directly dependent on deprecation's value?

I think this is the best option

nex3 · 2024-04-03T19:44:34Z

spec/js-api/deprecations.d.ts.md

+export interface Deprecations {
+```
+
+#### `call-string`


You could even get rid of the headers and have this all be a single code block.

nex3 · 2024-04-03T19:45:48Z

spec/js-api/deprecations.d.ts.md

+id: id;
+```
+
+The status of this deprecation.


Missing a header.

jathak added 5 commits April 1, 2024 05:42

WIP deprecations squashed onto new history

06426f4

Actually add deprecations.d.ts, misc changes

8869266

Update embedded protocol changelog

4b76f28

Fix proposal changelog

0f1186a

Add new deprecations to spec

590c264

This was referenced Apr 1, 2024

Add tests for the deprecations API sass/sass-spec#1967

Merged

Complete implementation the deprecations API sass/dart-sass#2207

Merged

Implement the host side of the deprecations API sass/embedded-host-node#279

Merged

jathak added 4 commits April 1, 2024 18:40

Fix toc and missing ```

672ca31

More fixes

11f8ee0

Rename deprecations proposal to avoid duplicate definitions

1a42270

More fixes

fc4e3c7

jathak requested a review from nex3 April 1, 2024 21:36

nex3 requested changes Apr 2, 2024

View reviewed changes

nex3 reviewed Apr 2, 2024

View reviewed changes

jathak added 2 commits April 3, 2024 00:08

Code review

510801e

Update warn options type in spec

bd2a1e9

jathak requested a review from nex3 April 3, 2024 02:07

nex3 approved these changes Apr 3, 2024

View reviewed changes

jathak added 2 commits April 3, 2024 19:56

Combine code blocks; fix header

b0af8d5

Fix toc

340a7f7

jathak merged commit ede7f25 into main Apr 3, 2024

jathak deleted the deprecations branch April 3, 2024 21:14

	* @param options.dperecationType - The type of deprecation this warning is
	* @param options.deprecationType - The type of deprecation this warning is

		> Interfaces for user-declared importers that customize how Sass loads
		> stylesheet dependencies.

		The compiler version this feature was fully removed in, making the deprecation
		obsolete.

Conversation

jathak commented Apr 1, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nex3 commented Apr 2, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants