-
-
Notifications
You must be signed in to change notification settings - Fork 113
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As reported by CodeQL, though unrelated to the rest of the PR
@@ -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'.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 possiblynull
, and for theget()
andgetIn()
methods returningunknown
. While technically accurate, these types are often a hindrance. To address this, let's add a second optional generic type argumentStrict
that relaxes these and a few other types:Add
undefined
&Date
handling to theNodeType<T>
utility typeExport
FoldOptions
type from'yaml/util'
Convert all remaining JS tests to TS & include their type checking in the
test:types
scriptUse
package.json
"typesVersions"
to find d.ts files fromdist/
Making the typing of
doc.directives
(which could technically benull
, but practically never is) depend onStrict
ended up hitting an issue with TS 4.2 and older, which provided this useful and very sensible error: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 theDocument.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: