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

Typescript declarations #46

Open
sbliven opened this issue Jul 12, 2018 · 15 comments · May be fixed by #62
Open

Typescript declarations #46

sbliven opened this issue Jul 12, 2018 · 15 comments · May be fixed by #62

Comments

@sbliven
Copy link

sbliven commented Jul 12, 2018

I am interested in using color-space in a typescript project. To this end I have created type declarations for the library (https://github.com/sbliven/chromanaut/blob/master/color-space.d.ts). I would like to make these available to other projects. This can be done in a few ways depending on the desired integration with the javascript color-space package.

  1. Include index.d.ts in the package directory. This has the benefit of keeping the type definitions and the javascript in the same repository, so the versions should always match. However, it would require that new pull requests modify the index.d.ts file to reflect any changes to the javascript API.

  2. Type declarations can be included in the DefinitelyTyped repository. This makes the process independent of the maintainers here, but runs a greater risk that the type information will be out of date.

Both methods are widely used. I'm happy to help maintain the declarations regardless of which approach is preferred by the community here.

@dy
Copy link
Member

dy commented Jul 12, 2018

@sbliven thanks for the interest. Sure, method 1 will work well, I will be glad to approve the PR.

@nstringham
Copy link

is there an update on this?

@dy
Copy link
Member

dy commented Jul 1, 2021

@nstringham PR welcome.

@sbliven
Copy link
Author

sbliven commented Jul 7, 2021

I haven't worked on my chromonaut project in years now. I will probably not get around to a PR unless someone really needs it. I guess my previous definition file will need to be updated somewhat.

@dy
Copy link
Member

dy commented Jan 30, 2025

There's an attempt at providing types that unfortunately doesn't seem was very successful, so I will put some R&D here.

@ahocevar your approach enables intellisense, but it doesn't solve this:

Image

Consuming scenarios:

  • Modular imports import rgb from 'color-space/rgb' in node, TS, browser with sourcemap, bundlers
  • Modular imports import rgb from 'color-space/rgb.js' in any ESM environment
  • Full imports import {rgb} from 'color-space' in node, TS, browser, anywhere
  • Local imports eg. import rgb from './rgb.js'

CJS is out of equation.

Ways to define types:

  1. Convert all files to .ts, as in Add typescript types #57
  • enables native types, but it's not minimal no-bs JS lib anymore
  • besides one day JS may introduce erasable types so .ts won't be necessary
  1. Single dist/color-space.d.ts declaration
  • doesn't seem to be able to define submodules nicely: declare module "color-space/cmy", "./cmy", "./cmy.js" don't work
  • requires aliases for various ways of importing
  1. types/*.d.ts multiple declarations
  • too many files
  • somehow same issue as with 2.
  1. /*.d.ts next to regular files
  • it works as 1. but clutters code
  1. @DefinitelyTyped/color-space/*.d.ts
  • whole separate package to maintain
  • solves the types definition without contaminating the lib

It seems 5. is the most reasonable way.

@ahocevar
Copy link
Contributor

Option 3 will work fine with the dist/color-space.d.ts file that #60 generates with npm run prepublishOnly. For both the default import as well as individual module imports.

@dy
Copy link
Member

dy commented Jan 30, 2025

@ahocevar see what it generates.
Besides it doesn't provide types properly. I still see the issue - intellisense uses factual JS props, not types info.
Image
If you could actually test and confirm if current master works on your side, because it doesn't work for me.

@ahocevar
Copy link
Contributor

Like I said - current master referenced in a project as npm dependency does NOT work, because of the missing dist/color-space.d.ts file. You can easily test that by running npm link in the master branch of this repo, and then try it in a different project where you had previously installed color-space, and run npm link color-space.

@ahocevar
Copy link
Contributor

ahocevar commented Jan 30, 2025

Sorry, I just saw you pushed another commit to master. I'm testing right now.

@ahocevar
Copy link
Contributor

Update: master now provides correct types in dist/:

Image Image

@ahocevar
Copy link
Contributor

ahocevar commented Jan 30, 2025

But you're right: individual module imports have missing types:

Image

And the reason for that is the missing types entry in package.json. You need to add

  "types": "./dist/color-space.d.ts",

That would have been solved with #59, i.e. with individual .d.ts files.

Another solution would be to define "exports" in package.json and point to individual .d.ts files in the dist/ (or a separate types/ folder for "types".

@ahocevar
Copy link
Contributor

@dy See #62 for everything working with types an type sourcemaps.

@ahocevar
Copy link
Contributor

ahocevar commented Jan 30, 2025

So, to summarize: what @sbliven suggested in 2018 (!) was basically what @MoonE contributed in #57. It was not merged, despite being a solid contribution. The .d.ts files could also have been published as @types/color-space, which also did not happen.

#62 now suggests a solution similar to option 1 suggested by @sbliven. It works well, but @dy does not like the JSDoc comments any more (merged with #58), which are used to auto-generate the types. @dy states also that they do not like to bring in any tooling.

Fun fact: The tooling used in #62 is just the TypeScript compiler, nothing else. On the other hand, @dy uses jsbuild (previously rollup) for auto-generating the dist/ directory - tooling which is totally unnecessary because the artifacts in dist/ are redundant: Every esm capable bundler and node (with "type": "module") can consume src/index.js and all other source modules directly. What's in dist/ can also only be consumed by esm capable bundlers and node (with "type": "module").

@dy
Copy link
Member

dy commented Jan 30, 2025

I am still open to making #57 a @types/color-space. If you want to do that @ahocevar - please make a favour. I just don't have time span atm, but I can also do that later.
Yes, I am not happy about all that tooling, you can see also #62 (comment). We use esbuild as minimal available bundler, we still need to provide single-file entry. You are right, it must be done as standalone version.

@dy
Copy link
Member

dy commented Jan 31, 2025

Ok, I started work on manual type declarations here.

Since https://github.com/DefinitelyTyped/DefinitelyTyped is monorepo, we have to store these declarations somewhere, so /types seems to be proper location. We cannot generate types from jsdoc unfortunately, since produced .d.ts files look dirty, besides jsdoc has limitations, like interfaces are unable to be merged, and we need it.

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 a pull request may close this issue.

4 participants