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

Type error: Expected 0 arguments, but got 1 on Color constructor #561

Open Southclaws opened 2 weeks ago

Southclaws commented 2 weeks ago

Super odd one here, on Vercel I see this error:

./src/components/form/ColourInput/useColourInput.ts:44:32
Type error: Expected 0 arguments, but got 1.
  42 |
  43 |     try {
> 44 |       const colour = new Color(props.value);
     |                                ^
  45 |
  46 |       const hue = angleToHue(colour.lch["h"] ?? 0);
  47 |
error Command failed with exit code 1.

And if I ignore this with some as any nonsense, the other part of the codebase that uses this lib shows:

Property 'to' does not exist on type 'Color'

So it seems something is wrong with either the way it's packaged or being imported - it seems to be pulling some other type that isn't what the import statement is pulling.

But locally, totally fine - no issues, builds fine, type-checks fine, etc.

Source code here: https://github.com/Southclaws/storyden/blob/main/web/src/components/form/ColourInput/useColourInput.ts#L3

This appears to be the only library affected in this codebase so I'm not sure what's happening. I've also tried downgrading to 0.4.5 but still doesn't work.

Unfortunately this is preventing progress so I might just have to remove color.js until a fix is found.

LeaVerou commented 2 weeks ago

Could this be the same bug as #560 ?

If so, the OP posted a workaround here.

@MysteryBlokHed @jgerigmeyer would it make sense to make a patch v0.5.1 release to fix this? Obviously we'd have to do it in a separate branch at this point.

MysteryBlokHed commented 2 weeks ago

@MysteryBlokHed @jgerigmeyer would it make sense to make a patch v0.5.1 release to fix this? Obviously we'd have to do it in a separate branch at this point.

Probably not a bad idea. I can make a branch off of release 0.5.0 and make the changes from https://github.com/color-js/color.js/issues/560#issuecomment-2182819685

LeaVerou commented 2 weeks ago

@MysteryBlokHed @jgerigmeyer would it make sense to make a patch v0.5.1 release to fix this? Obviously we'd have to do it in a separate branch at this point.

Probably not a bad idea. I can make a branch off of release 0.5.0 and make the changes from #560 (comment)

If you commit that patch, make sure to use a Co-Authored By with the person that came up with the workaround. I’ll defer to you as to whether it’s a good way to fix this.

MysteryBlokHed commented 2 weeks ago

Just created a branch to add the suggested change: https://github.com/color-js/color.js/tree/types/fix-new-ts

It's weird to me that this is necessary to fix the issue, but I don't think the change should cause any problems.

LeaVerou commented 2 weeks ago

LGTM. I’d name the branch after the version (v0.5.1) otherwise if we see a fix-new-ts branch 4 years later, we'll have no idea what "new-ts" means. Let me know when I should npm publish. Or what your npm username is, so I can give you permission to publish.

MysteryBlokHed commented 2 weeks ago

Renamed the branch to v0.5.1-fix so it doesn't conflict if we want to make a v0.5.1 tag. I think it's all ready to publish 😄

LeaVerou commented 2 weeks ago

@MysteryBlokHed do you want to do the honours? :D

MysteryBlokHed commented 2 weeks ago

Sure! Should we also create a release on GitHub to go along with it?

MysteryBlokHed commented 2 weeks ago

Release has been published :tada:

jorenbroekema commented 2 weeks ago

It seems to me the new release is also broken: image

When I do:

const backgroundColor = new Color('#000');
const contrastBlack = Math.abs(
  backgroundColor.contrast('#fff', 'APCA'),
);

I get a type error saying contrast method does not exist on type Color

MysteryBlokHed commented 2 weeks ago

I wonder if the best solution might be to just remove the module augmentation altogether.

Everything defined there could simply be moved into color.d.ts, separated (with comments) into its own section to keep track of what's defined in which files.

MysteryBlokHed commented 2 weeks ago

I wonder if the best solution might be to just remove the module augmentation altogether.

Everything defined there could simply be moved into color.d.ts, separated (with comments) into its own section to keep track of what's defined in which files.

This also shouldn't break any compatibility as long as the proper types are still re-exported from index.d.ts.

MysteryBlokHed commented 2 weeks ago

I wonder if the best solution might be to just remove the module augmentation altogether.

Everything defined there could simply be moved into color.d.ts, separated (with comments) into its own section to keep track of what's defined in which files.

@LeaVerou If we do this, do you think it'd be best to just add to the v0.5.1-fix branch (maybe after renaming it if possible), or create another branch off of that one? Just for organizational purposes.

Southclaws commented 2 weeks ago

Amazing turnaround on this, thank you! I'll give it a try tonight and re-add the colour.js code I had to quickly replace (I wasn't aware it was specifically a TS v5 issue and I'm using some other tools that depend on v5)

LeaVerou commented 1 week ago

@MysteryBlokHed I would rename to v0.5.x which also takes care of clashes.

As for how to fix it, I trust you to pick the best way (possibly run it by @jgerigmeyer ? But also time is of the essence…)

MysteryBlokHed commented 1 week ago

@Southclaws v0.5.2 has just been released to (re)try fixing a similar issue. Could you take a look and see if it works when you have a moment? 😄