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

feat: generate ts types #637

Closed
wants to merge 1 commit into from
Closed

Conversation

achingbrain
Copy link
Member

Adds a generate-types command to generate *.d.ts files.

Largely inspired by @AuHau's work on achingbrain/uint8arrays#4

Adds a `generate-types` command to generate `*.d.ts` files.

Largely inspired by @AuHau's work on achingbrain/uint8arrays#4
@achingbrain
Copy link
Member Author

achingbrain commented Sep 11, 2020

Oh yeah, there's also #568 which I forgot existed. Would be good to get something shipped here.

}

const forwardOptions = argv['--'] ? argv['--'] : []
const args = ['-d', '--emitDeclarationOnly'].concat(forwardOptions).concat(files)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should not we be passing a --declarationDir as well or is it expected to be passed as part of -- ?

Any flags or config to pass to `tsc` can be specified as forward options:

```console
$ aegir generate-types --overwrite 'src/**/*.js' -- --allowJs --esModuleInterop
Copy link
Contributor

Choose a reason for hiding this comment

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

Please note that tsconfig.json affects a lot what options can be passed or how they collide. E.g. having noEmit compiler option will prevent emitDeclarationOnly passed here.

I don't think documenting every single collision is realistic, but maybe a note or a link to an example setup might be helpful.

@Gozala
Copy link
Contributor

Gozala commented Sep 15, 2020

@achingbrain I think we need to decide between:

  1. Generating .d.ts files along the side of .js files like it's done in feat: ts definitions achingbrain/uint8arrays#4 and here.
  2. Genarating .d.ts files in a separate directory and have a typesVersions in package.json to tell other projects where to look those up. This is the approach used in feat: type check & generate defs from jsdoc js-ipfs#3281

My preference lays towards the 2nd option because:

  1. It allows having hand written .d.ts files if necessary. With the 1st option it gets messy with .gitignore and dir cleanups.
  2. Generating types with 1st option pollutes the source tree, while 2nd option does not. I think that is point worth considering especially with multipackage setup like in js-ipfs, because there you'll want to generate typedefs during development not just release so that interface changes e.g in ipfs-core-utils will be picked up by ipfs and all the mismatches will be reported.

I can also take over this work so I can align / coordinate it with ipfs/js-ipfs#3281

@hugomrdias
Copy link
Member

typeVersions is a thing now??? wow nice!!!

@achingbrain
Copy link
Member Author

I think we need to decide between:

I'm always slightly annoyed by VSCode when I try to click through to source code and it takes me to a type definition nowhere near the actual code that's being executed. The reason I'm looking for source code is because I either don't understand what a function is doing, or I think there's a bug in the function - type defs don't help me with either problem.

If the defs are next to the actual implementation it's at least slightly less irritating. Maybe there's a better way here, I don't know. TBH I'm sort of getting used to click through being broken.

As for the polluting the source tree thing, my thinking here was to only generate .d.ts files in a pre-publish step as we're not trying to convert these projects to ts, only to allow ts projects to have types available when requireing a module.

If the source tree pollution is so bad we can always add a postpublish script to delete the .d.ts files.

I'd rather not have hand-crafted .d.ts files if we can avoid it because they will get out of date and it shifts the responsibility of maintenance onto our contributors who may not know or care that much about TypeScript.

you'll want to generate typedefs during development

Eh, no, not really, I'd rather treat it like another linting step, but that's my personal preference.

I can also take over this work so I can align / coordinate it with ipfs/js-ipfs#3281

Please do. I was going to close this PR in favour of #568 as it seems to be a more complete solution but if you think there's value here please do what's necessary.

@hugomrdias
Copy link
Member

100% +1 on @achingbrain comment

@Gozala you said you figured out cmd+click to source thing here https://github.com/Gozala/web-blob/pull/7#issuecomment-660722815

@Gozala
Copy link
Contributor

Gozala commented Sep 16, 2020

I'm always slightly annoyed by VSCode when I try to click through to source code and it takes me to a type definition nowhere near the actual code that's being executed. The reason I'm looking for source code is because I either don't understand what a function is doing, or I think there's a bug in the function - type defs don't help me with either problem.

If the defs are next to the actual implementation it's at least slightly less irritating. Maybe there's a better way here, I don't know. TBH I'm sort of getting used to click through being broken.

There is now typedefs maps kind of like source maps that supposedly solve that. In my tests it seemed to work but they are not widely used yet.

@hugomrdias hugomrdias mentioned this pull request Nov 12, 2020
@hugomrdias hugomrdias deleted the feat/generate-ts-types branch February 23, 2021 15:27
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.

3 participants