Page MenuHomePhabricator

Use of modern TypeScript (syntax)
Closed, ResolvedPublic

Description

Ever since bridge development started (around TS 3.6 3.2) multiple new versions of TypeScript have been release, which offer new language features.

AC

  • Work through the release notes (3.7, 3.8, 3.9; to some degree also 3.3, 3.4, 3.5, 3.6), identify notable changes (e.g. "optional chaining")
  • apply changes to the bridge code base, clustered by feature change (e.g. change all places at once where "optional chaining" can help us)

Identified features to try (see thoughts in the comments)

Event Timeline

Discussion from "task break-down":

  • update to latest typescript
  • maybe find a tool which can highlight improvements which are possible
  • splitting this to multiple people might be a challenge/not useful as files may possibly touched by more than one patch and it would help for one contributor to have a good current overview of what the code base actually looks like
  • even if we don't find "all" places that could be fixed, by uploading a change for one pattern we inspire the others to also fix similar spots in the future

I think the notable changes are:

  • optional chaining: foo?.bar?.baz
  • nullish coalescing: foo ?? bar()
  • type-only imports+exports: import type Foo from "foo"; I think we have to decide whether we want to use these instead of regular imports (and potentially enforce that with "importsNotUsedAsValues": "error") or not; I like the idea but don’t feel very strongly about it
  • // @ts-expect-error: compile-time assert that the next line is an error; might be useful in some of our tests?

That’s all I could see; does anyone else also want to go through the release notes to check if I missed anything?

I may have been a fool when writing the description - looking at the package.json, we in fact started with TS 3.2. So maybe there are more goods in the changelog for those versions.

Otherwise the list looks sane but obviously I don't know the changes by heart.
One thing that struck with me when I read it (some time ago) were "assertion signatures" where I could not help but think that it may make some of the API code simpler, protecting us from invariants.

Thanks! I spotted additionally:

And not much else – it seems like those releases had more features about the way type checking works and performance, less in the way of new syntax we could use.

Regarding assertion signatures, I left them out of my list because I didn’t remember any assertion functions we had, and so I thought the ability to correctly type them wasn’t very important. But maybe we want to refactor some of our current type checking functions (returning x is Type), when they are called only to throw an error if they don’t match, into assertion funtions?

  • the Omit helper type: Omit<TypeName, "memberName">; I think I introduced a duplicate of that in MwApiParameters, something like MwApiParamsWithout?

Yeah, it’s MwApiParametersWithout<K extends string> – but it looks like using Omit<MwApiParameters, K> rather than MwApiParameters & { [ k in K ]?: never } results in no more errors being reported for assertCurrentUser( { assert: 'user' } ) calls, so I guess it doesn’t do the same thing. Nevermind.

All looks very nice. Hoisting so that people can add their name if they want to pick one (short of creating subtasks).

I’ll pick the readonly part, since it seems relatively easy to grep for that.

Change 599353 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] bridge: use TypeScript readonly arrays

https://gerrit.wikimedia.org/r/599353

Change 599928 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] bridge: use ?. and ?? operators in more places

https://gerrit.wikimedia.org/r/599928

Change 599929 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] bridge: change some type guards to assertion functions

https://gerrit.wikimedia.org/r/599929

Change 599928 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] bridge: use ?. and ?? operators in more places

https://gerrit.wikimedia.org/r/599928

Change 599353 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] bridge: use TypeScript readonly arrays

https://gerrit.wikimedia.org/r/599353

Change 599929 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] bridge: change some type guards to assertion functions

https://gerrit.wikimedia.org/r/599929

Change 602073 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] bridge: update to TypeScript 3.9

https://gerrit.wikimedia.org/r/602073

Change 602073 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] bridge: update to TypeScript 3.9

https://gerrit.wikimedia.org/r/602073

Lucas_Werkmeister_WMDE claimed this task.

I think we can close this now, it sounds like there’s not much enthusiasm for type-only imports.

I just found an analysis of TypeScript 3.8 that I’d done in Ie307d3ea2b:

… I don’t think there are a lot of interesting features for us in that release:

  • Type-Only Imports and Exports might be interesting if we want to use them, but we don’t have to.
  • ECMAScript Private Fields can’t be used on ES5 targets.
  • export * as ns Syntax is uninteresting since we don’t use that style.
  • Top-Level await is uninteresting since we don’t have top-level code (and also can’t be used on ES5 targets yet).
  • JSDoc Property Modifiers don’t matter to us since we don’t directly import much JS code without declaration files, I believe.
  • The other two features are compiler options.

Seems to more or less match what we found in this task.