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

Working TypeScript type definitions for both default and module imports #62

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ahocevar
Copy link
Contributor

Fixes #46

@ahocevar
Copy link
Contributor Author

The types in the types/ directory are auto-generated with npm run types.

Copy link
Member

@dy dy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahocevar so much work is done! I wonder what really stops us from maintaining a separate package? I really want to remove all comments and tooling from code. I believe it must be self-explanatory.
What's the main reason we are not doing @types/color-space?

@dy
Copy link
Member

dy commented Jan 30, 2025

I really don't want to deal with typescript in this package, I am sorry.

@ahocevar
Copy link
Contributor Author

You'll see it won't get easier with @types/color-space. You need more tooling and you'll have to maintain two packages. Just my 2 cents.

@ahocevar
Copy link
Contributor Author

I really don't want to deal with typescript in this package, I am sorry.

It would have been a more pleasant contributing experience if that would have been clear from the start. Looking at #57, where @MoonE suggested going that route, that didn't seem to satisfy you either. At least nothing was done on your end to complete or support the effort.

Now you've wasted both @MoonE's and my time. Time that would have been spent better contributing to other packages out there that gives us the features of color-space and color-rgba we need in https://github.com/openlayers/openlayers.

Very disappointed.

@dy
Copy link
Member

dy commented Jan 30, 2025

@ahocevar I wasn't sure about that from the beginning, I hoped it could be kept clean and simple, like a single separate color-space.d.ts file that we can manually edit. Now it doesn't look like that at all, and the trend code gets is horrifying.

Eg. the code style I aspire for is:

import rgb from './rgb.js';

const cmy = {
	name: 'cmy',
	min: [0, 0, 0],
	max: [100, 100, 100],
	channel: ['cyan', 'magenta', 'yellow'],
	alias: ['CMY']
};

cmy.rgb = ([c, m, y]) => [
	(1 - c/100) * 255,
	(1 - m/100) * 255,
	(1 - y/100) * 255
];


rgb.cmy = ([r, g, b]) => [
	(1-r/255) * 100 || 0,
	(1-g/255) * 100 || 0,
	(1-b/255) * 100 || 0
];


export default cmy;

It is minimal and self-explanatory. There's nothing to take away, which is (one of) definition of perfection.

I am sorry you spent your time - believe it or not I also sacrificed hours on that, and I have other projects to focus on.
That's the main reason I don't want to deal with TS - it consumes times the time project itself does.

One thing I don't understand yet - what makes you want to contribute here instead of @types/color-space? It seems #57 has half the job done. I can try doing it later.

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

Successfully merging this pull request may close these issues.

Typescript declarations
2 participants