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

Improve TS developer experience #441

Merged
merged 20 commits into from Feb 15, 2023
Merged

Improve TS developer experience #441

merged 20 commits into from Feb 15, 2023

Conversation

eemeli
Copy link
Owner

@eemeli eemeli commented Feb 13, 2023

This packages in a small bunch of fixes discovered while porting the remaining JS tests to TS:

  • Add Strict type flag to Composer, Document, parseDocument, and parseAllDocument

    The current typing of Document members and methods is pretty strict, as they account for e.g. doc.contents being possibly null, and for the get() and getIn() methods returning unknown. While technically accurate, these types are often a hindrance. To address this, let's add a second optional generic type argument Strict that relaxes these and a few other types:

    import { parseDocument, YAMLMap } from 'yaml'
    
    const strictDoc = parseDocument<YAMLMap>('foo: bar')
    strictDoc.contents // YAMLMap | null
    strictDoc.get('foo') // unknown
    
    const looseDoc = parseDocument<YAMLMap, false>('foo: bar')
    looseDoc.contents // YAMLMap
    looseDoc.get('foo') // any
  • Add undefined & Date handling to the NodeType<T> utility type

  • Export FoldOptions type from 'yaml/util'

  • Convert all remaining JS tests to TS & include their type checking in the test:types script

  • Use package.json "typesVersions" to find d.ts files from dist/

Making the typing of doc.directives (which could technically be null, but practically never is) depend on Strict ended up hitting an issue with TS 4.2 and older, which provided this useful and very sensible error:

Type 'boolean' is not assignable to type 'T extends true ? boolean | undefined : boolean'.

As I could not find any better workaround for this, the Document.d.ts file needs a /** @ts-ignore */ comment to deal with it, but that's unfortunately not parsed right in TS 3.8 and older (only JSDoc comments get added to d.ts files). It might be possible to fix this there via "typesVersions", but as far as I can tell that would require duplicating all of the 75 d.ts files just to be able to apply a single-line patch to the Document.d.ts as the version-dependent paths are only checked for entry points, and we're providing nearly everything from 'yaml'.

So instead this PR moves the minimum support level to TS 3.9 and adds a note to the readme and docs intro about this state of affairs:

The minimum supported TypeScript version of the included typings is 3.9; for use in earlier versions you may need to set skipLibCheck: true in your config. This requirement may be updated between minor versions of the library.

@@ -211,7 +220,7 @@
if (end) {
value = value.slice(0, -end.length)
if (end[end.length - 1] === '\n') end = end.slice(0, -1)
end = end.replace(/\n+(?!\n|$)/g, `$&${indent}`)
end = end.replace(blockEndNewlines, `$&${indent}`)

Check failure

Code scanning / CodeQL

Polynomial regular expression used on uncontrolled data

This [regular expression](1) that depends on [library input](2) may run slow on strings with many repetitions of '\n'. This [regular expression](1) that depends on [library input](3) may run slow on strings with many repetitions of '\n'. This [regular expression](1) that depends on [library input](4) may run slow on strings with many repetitions of '\n'. This [regular expression](1) that depends on [library input](5) may run slow on strings with many repetitions of '\n'. This [regular expression](1) that depends on [library input](6) may run slow on strings with many repetitions of '\n'. This [regular expression](1) that depends on [library input](7) may run slow on strings with many repetitions of '\n'. This [regular expression](1) that depends on [library input](8) may run slow on strings with many repetitions of '\n'. This [regular expression](1) that depends on [library input](9) may run slow on strings with many repetitions of '\n'. This [regular expression](1) that depends on [library input](10) may run slow on strings with many repetitions of '\n'. This [regular expression](1) that depends on [library input](11) may run slow on strings with many repetitions of '\n'. This [regular expression](1) that depends on [library input](12) may run slow on strings with many repetitions of '\n'. This [regular expression](1) that depends on [library input](13) may run slow on strings with many repetitions of '\n'. This [regular expression](1) that depends on [library input](14) may run slow on strings with many repetitions of '\n'. This [regular expression](1) that depends on [library input](15) may run slow on strings with many repetitions of '\n'. This [regular expression](1) that depends on [library input](16) may run slow on strings with many repetitions of '\n'. This [regular expression](1) that depends on [library input](17) may run slow on strings with many repetitions of '\n'.
@eemeli eemeli merged commit 6765cf5 into master Feb 15, 2023
@eemeli eemeli deleted the ts-fixes branch February 15, 2023 08:45
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.

None yet

1 participant