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

Add TypeScript typings #153

Merged
merged 22 commits into from
Apr 17, 2020
Merged

Add TypeScript typings #153

merged 22 commits into from
Apr 17, 2020

Conversation

eemeli
Copy link
Owner

@eemeli eemeli commented Mar 29, 2020

Fixes #102
Fixes DefinitelyTyped/DefinitelyTyped#33780
Closes DefinitelyTyped/DefinitelyTyped#43450
Related: #23, #47

These types are based on those published as @types/yaml, which are mostly the work of @ikatyang, with contributions from @ColinBradley and @shirk3y. In fact, I've picked out the original commits from DefinitelyTyped into this branch, and worked from there.

I'm relatively confident about these types, though I've not tested them very deeply. There are a few changes that may be relevant:

  • The DT types advertise themselves as being valid for TS 2.2; these require at the very least TS 2.3 due to Iterable, possibly a bit later than that.
  • Some of the interface and namespaces are available from different names and endpoints compared to before:
    // previously, with @types/yaml
    import { ast, cst, ParseOptions, YAMLError } from 'yaml'
    
    // now
    import { Options } from 'yaml'
    import { AST, YAMLError } from 'yaml/types'
    import { CST } from 'yaml/parse-cst'
  • The TS class inheritance doesn't exactly match the JS implementation, but it's really close.
  • I've not included typings for deprecated endpoints

The tsconfig.json and yaml-tests.ts files are remnants of the DT history, and should probably get removed before merging this.

Comments are very welcome here, in particular wrt changes to the DT types. For example, if there's reason to do so, re-exports could be added to match the previous typings.

@eemeli eemeli added the enhancement New feature or request label Mar 29, 2020
Copy link
Contributor

@ikatyang ikatyang left a comment

Choose a reason for hiding this comment

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

I tested it in prettier/yaml-unist-parser@d999748, it looks good but it seems you forgot to add files: ["*.d.ts"] in package.json.

Some of the interface and namespaces are available from different names and endpoints compared to before

I'd recommend to export all the used types. For example, index.d.ts exports parseCST from parse-cst.d.ts so all the types that are related to parseCST would be better to be re-exported to index.d.ts as well.

index.d.ts Show resolved Hide resolved
/**
* Used by some tags to configure their stringification, where applicable.
*/
options?: object
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we define interface for these options?

Copy link
Owner Author

Choose a reason for hiding this comment

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

This needs to be extendable by custom tags, each of which will have their own options shape. I'm not sure what specifying this further would really help?

@eemeli
Copy link
Owner Author

eemeli commented Apr 1, 2020

@ikatyang I added re-exports of the AST and CST namespaces to index.d.ts. All of the other types are tied pretty closely to the actual exports of yaml/types and yaml/util, so I don't think they really make sense being re-exported. Also good point about actually include the declarations in the package.

@eemeli eemeli merged commit 663acc1 into master Apr 17, 2020
@eemeli eemeli deleted the add-dts branch April 17, 2020 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeScript definitions are out of date [@types/yaml] some accessor methods missing
3 participants