Skip to content
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

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

Closed
Southclaws opened this issue Jun 23, 2024 · 17 comments
Closed

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

Southclaws opened this issue Jun 23, 2024 · 17 comments

Comments

@Southclaws
Copy link

Southclaws commented Jun 23, 2024

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
Copy link
Member

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
Copy link
Member

@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)

@LeaVerou
Copy link
Member

@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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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

@MysteryBlokHed
Copy link
Member

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

@MysteryBlokHed
Copy link
Member

Release has been published 🎉

@jorenbroekema
Copy link

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

@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
Copy link
Member

@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? 😄

@briandonahue
Copy link

@MysteryBlokHed 0.5.2 seems to fix it for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants