color-js / color.js

Color conversion & manipulation library by the editors of the CSS Color specifications
https://colorjs.io
MIT License
1.81k stars 80 forks source link

[types] Remove module augmentation (v0.6) #567

Closed MysteryBlokHed closed 1 week ago

MysteryBlokHed commented 1 week ago

This is essentially the same change as #566, but for the main branch instead of a patch for v0.5.

I don't think there should be anything missing here—I re-copied and pasted the augmentation contents from index.d.ts to color.d.ts to make sure I didn't miss any new properties—but review has been requested just in case :)

netlify[bot] commented 1 week ago

Deploy Preview for colorjs failed. Why did it fail? →

Name Link
Latest commit 16328cfb0bbc6807882508ab687364c83f89069a
Latest deploy log https://app.netlify.com/sites/colorjs/deploys/667cbb7e6b59ee000846e6c7
lloydk commented 1 week ago

LGTM

I tested it with #569 and all the type tests passed.

lloydk commented 1 week ago

I don’t think build:ts is being run. On my phone with no reading glasses so I could be wrong. If that isn’t it then I’ll take a look when I get home.

On Thu, Jun 27, 2024 at 9:45 AM Jonny Gerig Meyer @.***> wrote:

@.**** commented on this pull request.

These specific changes look good, but it looks like since #564 https://github.com/color-js/color.js/pull/564 was merged, the entire build process is failing because of errors in the tsc step. Even combining this with #569 https://github.com/color-js/color.js/pull/569 doesn't help, so it's difficult for me to test. What am I missing?

— Reply to this email directly, view it on GitHub https://github.com/color-js/color.js/pull/567#pullrequestreview-2145795901, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAP4MCQLL75RE6AIUW3LYDZJQXQBAVCNFSM6AAAAABJ65FWNSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNBVG44TKOJQGE . You are receiving this because your review was requested.Message ID: @.***>

MysteryBlokHed commented 1 week ago

tsc is probably reporting type errors from the actual logic of the code. That should probably actually be fixed within the code, but in the meantime, is there some way to force it to exit with code 0? Just so we can actually get the build to run.

lloydk commented 1 week ago

tsc is probably reporting type errors from the actual logic of the code. That should probably actually be fixed within the code, but in the meantime, is there some way to force it to exit with code 0? Just so we can actually get the build to run.

I created #571 that should allow the build to run.

MysteryBlokHed commented 1 week ago

I'll try merging this and the other active typing PR's and see what the situation is like then