-
Notifications
You must be signed in to change notification settings - Fork 82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[types] Merge Color
types
#259
[types] Merge Color
types
#259
Conversation
The types originally had two definitions for the `Color` type which weren't interchangeable. This PR changes the types to more accurately reflect the original JS by augmenting the existing `Color` type when `src/index.d.ts` is imported. There is a problem with how I've done this right now: the module augmentation is happening even when the normal `src/color.d.ts` version of the class is imported. I'm not sure how this is happening without `src/index.d.ts` even being referenced, so I'll have to debug that when I get the chance.
I can't help much with the TS side of things, but thank you so much for working on this! |
I completely forgot I opened this PR 😅 The only solution I can think of is to merge this as-is. The only problem with that is it will cause properties that only exist when |
I’m about to release a new version, should I merge this in? |
I think it's ready to be merged. The only problem I noted is that some properties that only exist after importing |
Yeah, TS doesn't do that well with very dynamic JS. I guess it's better if they always exist than if they never exist. |
Closes #258
The types originally had two definitions for the
Color
type which weren't interchangeable. This PR changes the types to more accurately reflect the original JS by augmenting the existingColor
type whensrc/index.d.ts
is imported.There is a problem with how I've done this right now: the module augmentation is happening even when the normal
src/color.d.ts
version of the class is imported. I'm not sure how this is happening withoutsrc/index.d.ts
even being referenced, so I'll have to debug that when I get the chance. Any help to make sure the augmentation only happens whensrc/index.d.ts
is imported would be appreciated.You can look at the failing test to see the problem: https://github.com/LeaVerou/color.js/pull/259/files#diff-5bb8dd015c2660f438205df14840aa404517154744bcf054146734f0d5f8ad26