Conversation
Contributor
Author
|
I'm basing this on the JS API doc branch so that you can review with documentation, but if it looks good I'll rebase onto main and update the tests and the embedded host. |
fe3196d to
67455d2
Compare
This was referenced Nov 12, 2021
Awjin
approved these changes
Nov 12, 2021
67455d2 to
2d925ae
Compare
2d925ae to
e7a659a
Compare
This takes more advantage of overloads rather than using factory constructors or methods with different names.
e7a659a to
a50c1bb
Compare
stof
suggested changes
Nov 16, 2021
| lightness: number, | ||
| alpha?: number | ||
| ): SassColor; | ||
| constructor(options: { |
Contributor
There was a problem hiding this comment.
Given that the 3 overloads all have a single argument and the same return type (none as this is a constructor), the idiomatic way to write such typescript types is to use a union type instead of overloads: https://www.typescriptlang.org/docs/handbook/declaration-files/do-s-and-don-ts.html#use-union-types
Contributor
Author
There was a problem hiding this comment.
As discussed elsewhere, constructor overloads are necessary here in order to disallow new Color({hue: ..., red: ...}).
| get alpha(): number; | ||
|
|
||
| changeRgb(options: { | ||
| change(options: { |
Contributor
There was a problem hiding this comment.
The idiomatic way to write this is also to use a union rather than overloads (btw, that's how you did it in embedded-host-node)
mirisuzanne
pushed a commit
that referenced
this pull request
Feb 10, 2022
This takes more advantage of overloads and unions rather than using factory constructors or methods with different names.
mirisuzanne
added a commit
that referenced
this pull request
Feb 10, 2022
* main: (149 commits) Add the sourceMapIncludeSources option in the new JS API (#3226) Fix the toc tool to insert the toc (#3225) Fix some incorrect documentation/typings in LegacyPluginThis.options (#3246) Add a type annotation for the top-level NULL field (#3243) Add type annotations for the top-level TRUE and FALSE fields (#3241) Add type annotations for the sass.types.Error class (#3238) Add a type declaration for LegacyPluginThis.options.context (#3236) Make LegacyAsyncFunction type more usable (#3237) Allow SassFunction signatures to be checked on return (#3220) Make LegacyFileOptions.data optional (#3215) Use the URL type from the DOM definitions, not from Node (#3214) Document the new JS API (#3183) Make the new JS API a bit more idiomatic (#3200) Update immutable-js dep. (#3195) Add a code example for Logger.silent (#3192) Update dependencies (#3193) Replace FileImporterResult with a plain URL (#3180) Convert explicit compatiblity HTML into a nicer @-tag Convert explicit "Heads up" HTML into nicer-looking Markdown Automatically handle absolute file: URLs for FileImporters (#3181) ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This takes more advantage of overloads rather than using factory
constructors or methods with different names.