Update `typescript` #216

ChristianMurphy · 2023-03-17T15:49:30Z

Initial checklist

I read the support docs
I read the contributing guide
I agree to follow the code of conduct
I searched issues and couldn’t find anything (or linked relevant results below)
If applicable, I’ve added docs and tests

Description of changes

First step towards moving most or all of the types into JSDoc.
Previously lack of overloads was a blocker.
TypeScript 5 includes overload syntax for JSDoc https://devblogs.microsoft.com/typescript/announcing-typescript-5-0/#overload-support-in-jsdoc

The full migration to JSDoc will be large, this is the first step towards that, ensuring the project still builds on TypeScript 5, and building it with TS5 by default.

ChristianMurphy · 2023-03-17T15:53:28Z

test/process.js

+        /**
+         * @param {Node} tree
+         * @param {VFile} file
+         */


these are needed by type coverage, tree was considered an implicit any

Was TypeScript able to infer its type before? This seems like a regression in TypeScript.

~~Good question, inside VSCode, both TS4.9 and TS5.0 show Node<Data> even without this annotation.~~
~~type-coverage is where the error appears~~
(correction, this is specific to TS5.0, see later comment)

unified/test/process.js:50:19: tree unified/test/process.js:103:19: tree unified/test/run.js:552:19: tree unified/test/run.js:562:19: _ unified/test/use.js:255:19: node 2855 / 2860 99.82%

Looks like TS5 support in type-coverage is really new plantain-00/type-coverage#118
I'm double checking I have version v2.25.0 or higher locally now.

It is version v2.25.0 ✅

They’re asking the community to report regressions. I think this one would be a most welcome issue upstream. :)

I would like to update to TypeScript 5, but unless we want to use any new TypeScript features, I’m not in a hurry to update. I’d rather wait for an upstream fix than adding workarounds.

Hmm, this is an interesting one, when I try to replicate the issue in the TS Playground, it works again.
(playground, playground 2 in either disable noUnusedLocals)

I would like to update to TypeScript 5, but unless we want to use any new TypeScript features, I’m not in a hurry to update. I’d rather wait for an upstream fix than adding workarounds.

💯 agree

Update, as of TypeScript 5.1.6 and Type Coverage 2.26.0, it is still an any type.
typescript does not throw anymore, but type-coverage does.

unified/test/process.js:50:19: tree 2865 / 2866 99.96% The type coverage rate(99.96%) is lower than the target(100%).

You can use ignoreFiles to allow needed anys. Like this: https://github.com/syntax-tree/hast-util-sanitize/blob/23295ffd46c9fd4c6de0707cb33b6a4af0d52b2d/package.json#L87-L90

codecov-commenter · 2023-03-17T15:53:50Z

Codecov Report

Patch and project coverage have no change.

Comparison is base (4a94722) 100.00% compared to head (c08667b) 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #216   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines          596       596           
=========================================
  Hits           596       596

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

tsconfig.json

wooorm · 2023-03-18T13:36:29Z

So this can be merged, and then TS may improve some things later in their project?

ChristianMurphy · 2023-03-21T14:47:11Z

So this can be merged, and then TS may improve some things later in their project?

That could be an option, thoughts @remcohaszing?

remcohaszing · 2023-03-21T14:57:48Z

Still this: https://github.com/unifiedjs/unified/pull/216/files#r1140496841

Updating now doesn't give us any benefits. It only means we need to apply a workaround for a regression. I'd rather delay until this regression is fixed upstream, or TypeScript adds new features that do benefit unified.

This is just a dev dependency. It doesn't matter for users.

wooorm · 2023-08-09T13:14:08Z

What’s the status of this @ChristianMurphy?

ChristianMurphy · 2023-08-09T13:39:37Z

What’s the status of this @ChristianMurphy?

Merged, apparently 🙂

Maybe updating `types/unist` will solve it? Related-to: #216.

TypeScript 5

91541b9

github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Mar 17, 2023

ChristianMurphy commented Mar 17, 2023

View reviewed changes

tsconfig.json Show resolved Hide resolved

ChristianMurphy added the ☂️ area/types This affects typings label Mar 17, 2023

types remove lf option, it is default in ts5

521787d

ChristianMurphy commented Mar 17, 2023

View reviewed changes

tsconfig.json Show resolved Hide resolved

types enforce casing is now default enabled

f430744

ChristianMurphy commented Mar 17, 2023

View reviewed changes

tsconfig.json Show resolved Hide resolved

Merge branch 'main' into typescript-5

c08667b

wooorm changed the title ~~TypeScript 5~~ Update typescript Aug 9, 2023

wooorm merged commit ffc146c into unifiedjs:main Aug 9, 2023

wooorm added the 💪 phase/solved Post is done label Aug 9, 2023

This comment has been minimized.

Sign in to view

github-actions bot removed the 🤞 phase/open Post is being triaged manually label Aug 9, 2023

ChristianMurphy deleted the typescript-5 branch August 9, 2023 13:38

wooorm added a commit that referenced this pull request Aug 9, 2023

Remove unneeded explicit types in tests

250d3e5

Maybe updating `types/unist` will solve it? Related-to: #216.

InfiniteLove2020 mentioned this pull request Oct 3, 2024

[Snyk] Upgrade unified from 7.1.0 to 11.0.5 InfiniteLove2020/node#7

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update `typescript` #216

Update `typescript` #216

ChristianMurphy commented Mar 17, 2023

ChristianMurphy Mar 17, 2023

remcohaszing Mar 17, 2023

ChristianMurphy Mar 17, 2023 •

edited

Loading

ChristianMurphy Mar 17, 2023

ChristianMurphy Mar 17, 2023

remcohaszing Mar 17, 2023

ChristianMurphy Mar 17, 2023 •

edited

Loading

ChristianMurphy Mar 18, 2023

ChristianMurphy Aug 2, 2023

wooorm Aug 3, 2023

codecov-commenter commented Mar 17, 2023 •

edited

Loading

wooorm commented Mar 18, 2023

ChristianMurphy commented Mar 21, 2023

remcohaszing commented Mar 21, 2023

wooorm commented Aug 9, 2023

This comment has been minimized.

ChristianMurphy commented Aug 9, 2023

Update typescript #216

Update typescript #216

Conversation

ChristianMurphy commented Mar 17, 2023

Initial checklist

Description of changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristianMurphy Mar 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristianMurphy Mar 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Mar 17, 2023 • edited Loading

Codecov Report

wooorm commented Mar 18, 2023

ChristianMurphy commented Mar 21, 2023

remcohaszing commented Mar 21, 2023

wooorm commented Aug 9, 2023

This comment has been minimized.

ChristianMurphy commented Aug 9, 2023

Update `typescript` #216

Update `typescript` #216

ChristianMurphy Mar 17, 2023 •

edited

Loading

ChristianMurphy Mar 17, 2023 •

edited

Loading

codecov-commenter commented Mar 17, 2023 •

edited

Loading