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] Re-export types for `index` and `index-fn` #555

Closed MysteryBlokHed closed 1 week ago

MysteryBlokHed commented 3 weeks ago

Closes #551 Fixes a regression caused by #540

Re-exports types that were previously exported in index.js and index-fn.js.

If anyone knows a better way to re-export the index-fn types other than what I've done (a series of @typedef statements), I'm open to suggestions.

Also: note that the "re-exported from..." comments are currently actually incorrect—they come from the test files and indicate where the types used to be defined. I can remove or alter these comments if desired.

netlify[bot] commented 3 weeks ago

Deploy Preview for colorjs ready!

Name Link
Latest commit effc25f16c088db710079c0414a9e82b628cdcc4
Latest deploy log https://app.netlify.com/sites/colorjs/deploys/666dc85b7d7acd0008d41d36
Deploy Preview https://deploy-preview-555--colorjs.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

lloydk commented 3 weeks ago

I tried using this branch on my project but ran into some compilation errors.

image

It looks like the typescript compiler can't find the types. Do I need to change something in my typescript project to see the types?

MysteryBlokHed commented 3 weeks ago

Hm. I'll take a look in a few minutes, but it should just be using the JSDoc types of that file automatically. Maybe try adding "allowJs": true and/or "checkJs": true to your own tsconfig?

MysteryBlokHed commented 3 weeks ago

Hm. I'll take a look in a few minutes, but it should just be using the JSDoc types of that file automatically. Maybe try adding "allowJs": true and/or "checkJs": true to your own tsconfig?

If this works, then it might be worth using .d.ts again to avoid forcing TS consumers to do this. If I understand correctly, it should be possible to generate .d.ts files while using JSDoc types, so we can still keep the docs with the code as we wanted.

lloydk commented 3 weeks ago

Hm. I'll take a look in a few minutes, but it should just be using the JSDoc types of that file automatically. Maybe try adding "allowJs": true and/or "checkJs": true to your own tsconfig?

Tried adding each one individually and both together and had the same problem.

MysteryBlokHed commented 3 weeks ago

Did this happen for earlier commits on main? If so, then I have an idea:

The project's typing tests can resolve that path, which I think comes from the fact that this configuration only exists in the project's tsconfig:

"paths": {
    "colorjs.io": ["../src"],
    "colorjs.io/fn": ["../src/index-fn.js"],
    "colorjs.io/*": ["../*"]
}

I'm not sure whether there's a good way to reflect this in the package.json. Would you mind checking whether the problem exists on v0.5.0 (before the types were moved to JSDoc)? Maybe adding back some of the removed fields from the package.json would be enough.

If that's it, then I can open a separate PR to try and fix it.

lloydk commented 3 weeks ago

Did this happen for earlier commits on main? If so, then I have an idea:

The project's typing tests can resolve that path, which I think comes from the fact that this configuration only exists in the project's tsconfig:

"paths": {
  "colorjs.io": ["../src"],
  "colorjs.io/fn": ["../src/index-fn.js"],
  "colorjs.io/*": ["../*"]
}

I'm not sure whether there's a good way to reflect this in the package.json. Would you mind checking whether the problem exists on v0.5.0 (before the types were moved to JSDoc)? Maybe adding back some of the removed fields from the package.json would be enough.

If that's it, then I can open a separate PR to try and fix it.

Everything worked fine up to the big jsdoc commit. I've been running again https://github.com/color-js/color.js/tree/ca7391600ee85635c25515adf0b23be9660f7e3c for the last week.

I'll play around with adding some fields to package.json to see if i can get it to work.

lloydk commented 2 weeks ago

The only way I could get my project to compile was to add the "types" field back to package.json

        "./fn": {
            "import": {
                "default": "./src/index-fn.js",
                "types": "./src/index-fn.d.ts"
            },

And then use this hacked togethjer index-fn.d.ts file. I only added enough exports to get my project to compile so it doesn't include everything. I also had to export each module individually, export * didn't work for some reason.

In VSCode intellisense didn't work for files with jsdoc comments.

I suspect we may have to generate .d.ts files but I'm far from a Typescript export so maybe there's a way to get things working correctly.

I also had to change "moduleResolution" in my tsconfig.json from Node to bundler.

index-fn.d.ts ``` js // Type re-exports export { // Re-exported from src/color.d.ts ColorConstructor, ColorObject, ColorTypes, Coords, DefineFunctionCode, DefineFunctionOptions, DefineFunctionHybrid, PlainColorObject, SpaceAccessor, ToColorPrototype, // Re-exported from src/adapt.d.ts White, // Re-exported from src/CATs.d.ts CAT, // Re-exported from src/display.d.ts Display, // Re-exported from src/interpolation.d.ts Range, RangeOptions, MixOptions, StepsOptions, // Re-exported from src/parse.d.ts ParseOptions, // Re-exported from src/rgbspace.d.ts RGBOptions, // Re-exported from src/serialize.d.ts SerializeOptions, // Re-exported from src/space.d.ts Format as SpaceFormat, CoordMeta, Ref, SpaceOptions, } from "./types.js"; export {default as ColorSpace} from "./ColorSpace.js"; export {default as RGBColorSpace} from "./RGBColorSpace.js"; export {default as hooks, Hooks} from "./hooks.js"; export {default as defaults} from "./defaults.js"; export {default as getColor} from "./getColor.js"; export {default as get} from "./get.js"; export {default as getAll} from "./getAll.js"; export {default as set} from "./set.js"; export {default as setAll} from "./setAll.js"; export {default as parse} from "./parse.js"; export {default as to} from "./to.js"; export {default as serialize} from "./serialize.js"; export {default as display} from "./display.js"; export {default as inGamut} from "./inGamut.js"; export {default as toGamut, toGamutCSS} from "./toGamut.js"; export {default as distance} from "./distance.js"; export {default as deltas} from "./deltas.js"; export {default as equals} from "./equals.js"; export {default as contrast} from "./contrast.js"; export {default as clone} from "./clone.js"; export { getLuminance, setLuminance } from "./luminance.js"; export {uv, xy} from "./chromaticity.js"; export { default as contrastAPCA } from "./contrast/APCA.js"; export { default as contrastDeltaPhi } from "./contrast/deltaPhi.js"; export { default as contrastLstar } from "./contrast/Lstar.js"; export { default as contrastMichelson } from "./contrast/Michelson.js"; export { default as contrastWCAG21 } from "./contrast/WCAG21.js"; export { default as contrastWeber } from "./contrast/Weber.js"; export {default as XYZ_D65} from "./spaces/xyz-d65.js"; export {default as XYZ_D50} from "./spaces/xyz-d50.js"; export {default as XYZ_ABS_D65} from "./spaces/xyz-abs-d65.js"; export {default as Lab_D65} from "./spaces/lab-d65.js"; export {default as Lab} from "./spaces/lab.js"; export {default as LCH} from "./spaces/lch.js"; export {default as sRGB_Linear} from "./spaces/srgb-linear.js"; export {default as sRGB} from "./spaces/srgb.js"; export {default as HSL} from "./spaces/hsl.js"; export {default as HWB} from "./spaces/hwb.js"; export {default as HSV} from "./spaces/hsv.js"; export {default as P3_Linear} from "./spaces/p3-linear.js"; export {default as P3} from "./spaces/p3.js"; export {default as A98RGB_Linear} from "./spaces/a98rgb-linear.js"; export {default as A98RGB} from "./spaces/a98rgb.js"; export {default as ProPhoto_Linear} from "./spaces/prophoto-linear.js"; export {default as ProPhoto} from "./spaces/prophoto.js"; export {default as REC_2020_Linear} from "./spaces/rec2020-linear.js"; export {default as REC_2020} from "./spaces/rec2020.js"; export {default as OKLab} from "./spaces/oklab.js"; export {default as OKLCH} from "./spaces/oklch.js"; export {default as Okhsl} from "./spaces/okhsl.js"; export {default as Okhsv} from "./spaces/okhsv.js"; export {default as CAM16_JMh} from "./spaces/cam16.js"; export {default as HCT} from "./spaces/hct.js"; export {default as Luv} from "./spaces/luv.js"; export {default as LCHuv} from "./spaces/lchuv.js"; export {default as HSLuv} from "./spaces/hsluv.js"; export {default as HPLuv} from "./spaces/hpluv.js"; ```
LeaVerou commented 2 weeks ago

What does Svelte do? They use the same typing architecture, right?

LeaVerou commented 2 weeks ago

I suggest we merge this since we need to deal with the breakage ASAP, and investigate how we can improve on it later.

MysteryBlokHed commented 2 weeks ago

From a glance at their npm page, it seems that Svelte does generate .d.ts files:

Screenshot of svelte package on npm, showing bundled TypeScript defnitions

So we could do the same thing for color.js to fix the issue

MysteryBlokHed commented 2 weeks ago

I suggest we merge this since we need to deal with the breakage ASAP, and investigate how we can improve on it later.

Sounds good to me

LeaVerou commented 2 weeks ago

Fine by me, though we should report the issue to the TS folks in case it’s a bug.

lloydk commented 2 weeks ago

Svelte has a script to generate types.

https://github.com/sveltejs/svelte/blob/main/packages/svelte/scripts/generate-types.js

https://github.com/sveltejs/svelte/blob/main/packages/svelte/package.json

MysteryBlokHed commented 2 weeks ago

Just opened #563 for further discussion, since it's not really related to this PR

MysteryBlokHed commented 2 weeks ago

Should we go ahead and merge this PR now, and then think about a separate one for how to generate declarations?