open-props icon indicating copy to clipboard operation
open-props copied to clipboard

feat: Expose TypeScript declarations

Open aaronccasanova opened this issue 2 years ago • 4 comments

Do not merge: See TODO section below

This PR updates the lib:js build to additionally output TypeScript declarations. Microbundle already handles TypeScript projects with zero config and thus only a few tweaks were needed to emit declaration files:

  • Added a tsconfig.json defining the dist/types output directory
  • Updated the package.json "types": and "exports.types": keys to resolve the emitted dist/types/index.d.ts
  • Added the type-fest (TypeScript utility types package) as a dev dependency to address the any types in the src/index.js default export
  • Added a JSDoc annotation to help tsc understand the camel case keys transform on the default export

After running npm run lib:js

image

Example npm linking and using the intellisense in a .mjs file

image

Important

There is still one issue I haven't resolved and that is that the declarations in .cjs modules want to resolve values off OpenProps.default: image

TODO:

  • [ ] - Debug why .cjs files think props are accessible from OpenProps.default
  • [ ] - Take a second pass at the tsconfig compilerOptions and ensure the appropriate defaults have been set

aaronccasanova avatar Apr 15 '22 17:04 aaronccasanova

@argyleink Regarding the TypeScript issue in CommonJs modules. As shown below, TypeScript currently thinks tokens are accessible from OpenProps.default[token] (on the default export). Whereas, they are actually accessible directly from OpenProps (e.g. OpenProps.animationBlink):

image

I'm struggling to identify the exact issue, but have narrowed things down a bit. Microbundle by default sets the rollup.OutputOptions.exports to 'auto' which according to the output.exports section of the Rollup docs could be a factor if default option is inferred:

default – if you are only exporting one thing using export default ...; note that this can cause issues when generating CommonJS output that is meant to be interchangeable with ESM output, see below

I've tried various alterations to the Microbundle setup and tsconfig and only managed to get predictable results by switching src/index.js from a default export to a named export: e.g.

const { OpenProps } = require('open-props')
import { OpenProps } from 'open-props'

I validated this update by:

  • npm linking open-props to a local test-open-props-types project
  • Creating a .mjs and .cjs file that named import OpenProps and log OpenProps.animationBlink
  • Ran both scripts verifying the logs
  • Ran a type check with the minimum compilerOptions npx tsc <file> --noEmit --allowJs --checkJs

IMPORTANT: This would be a breaking change!

Hopefully, someone more familiar with microbundle and hybrid ESM/CommonJs packages can chime in to avoid introducing a breaking change. However, as mentioned above converting to named exports was the only success I've had.

aaronccasanova avatar Apr 16 '22 18:04 aaronccasanova

the JS api should support both object and array accessor styles, and I wasnt able to find a swift resolution in 15m. thoughts on this?

It's a quick update https://github.com/argyleink/open-props/pull/225/commits/bd34e2f39c9add0cf1afc04797cd20c5901b281f. For some reason I thought the original utility was replacing the dashed properties with camel case keys. The fix is simply intersecting the original inferred type T. I should probably rename keysToCamelCase to addCamelCasedProperties or leave the original mapToObjectNotation name.

image

I'd prefer to not create any breaking changes, let's continue iterating here to see what we can do.

That's what I figured, makes sense 👍 Might just have to introduce the typescript dev dependency and run tsc with the --emitDeclarationsOnly option. I'll play around with microbundle a bit more and otherwise fallback to tsc.

aaronccasanova avatar Apr 19 '22 03:04 aaronccasanova

Here's a repo I used to test autocomplete and imports for this PR:

https://github.com/hchiam-test-org/svelte-kit-typescript-open-props2/blob/main/my-app/package.json#L37 (WIP)

cf. https://github.com/argyleink/open-props/pull/223#issuecomment-1126590623

hchiam avatar May 14 '22 00:05 hchiam

@argyleink I haven't had the bandwidth to debug the ESM/CJS interop issues. Feel free to close the PR and/or @hchiam you're welcome to take it over!

aaronccasanova avatar Aug 13 '22 00:08 aaronccasanova

does this still need help? I would be happy to pick it up since I would love to get the Types working as first-class citizens

Jarvis1010 avatar Dec 09 '22 18:12 Jarvis1010

available in v1.5.4!

argyleink avatar Jan 24 '23 05:01 argyleink