Page MenuHomePhabricator

Type check JavaScript documentation
Closed, ResolvedPublic3 Estimated Story Points

Description

For documentation to be accurate, it must correspond to the code that is written. This task encompasses the work to verify the JSDocs are in synchronization with the code they document. There are _many_ explorations for why type checking is useful, however, given the precedent set by Wikimedia's Phan usage and WMDE's full TypeScript usage for Termbox, this task takes no effort to enumerate them.

Acceptance criteria

  • Revise and add existing JSDoc typing as needed. Avoid ignoring errors but keep patches digestible.
  • Add NPM test:types script and update test to invoke it. A bad type should block a Gerrit merge. (Edit: this ended up being a three letter command, tsc, which didn't seem worthy of adding an NPM script.)
  • Consider writing a similar task for Popups (migration may take longer given the size of the JavaScript codebase and missing Redux state typing).

QA

No functional changes so no QA is necessary.

Additional notes

  • Adding additional detail to JSDocs like descriptions for functions and their parameters is welcome but not required.
  • Add type definitions where needed to avoid any usage.
  • Consider adding WMDE to patch as they may have interest.

Read further

Event Timeline

@alexhollender, @ovasileva this is the type checking task I mentioned in retro to help prevent regressions. It's ready to work on and should be smallish given how little JavaScript is in Vector _right now_.

o/ @ovasileva, this task is ready for estimation and timely given @nray's T239258 work.

Niedzielski set the point value for this task to 3.

Change 575081 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/skins/Vector@master] [WIP] [JavaScript] Validate types

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

Change 575081 merged by jenkins-bot:
[mediawiki/skins/Vector@master] [JavaScript] Validate types

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

☝️ This is a technical change that only affects developer tooling and documentation, hence skipping *Needs QA*.

Volker_E updated the task description. (Show Details)
Volker_E added a subscriber: Sniedzielski.
Volker_E removed a subscriber: Sniedzielski.

Once more, Gerrit/Phab/Slack mish-mash :/

Consider writing a similar task for Popups (migration may take longer given the size of the JavaScript codebase and missing Redux state typing).

I would definitely be supportive of this, but in light of the team trying to reduce work outside desktop improvements I don't want to create work for us right now. From experience the typechecking is working pretty well in Vector, so let's focus on showing its worth there for now. I'm super grateful to stephen for writing his blog post to kick off this conversation. I see that Wikimedia Germany are already pushing the conversation by suggesting the addition of it to core which is very exciting - https://gerrit.wikimedia.org/r/c/mediawiki/core/+/585781