Add JSDoc enforcement + some sort of type checking for build-system/

[rsimha]

For the AMP runtime, we enforce JSDoc requirements for all code, and check types using closure compiler. However, build-system/ currently lacks JSDoc enforcement, and we do not type-check it using closure.

This issue is to track two things:

• Enforce JSDoc requirements and backfill missing documentation for existing code.
• Do some sort of type checking. If not closure, perhaps use typescript? Or failing that, maybe there's an eslint plugin that can do it?

[rsimha]

/cc @ampproject/wg-infra and @ampproject/wg-runtime for opinions / ideas.

[estherkim]

What if we wrote build-system in TypeScript?

[jridgewell]

Typescript would be cool, and you won't need to spin up a java process to typecheck. The Typescript language server can be spun up by your editor by default, and run the checks incrementally as you code.

[rcebulko]

Agree build system is a good target for Typescript. Explicitly defining some of the types might also make obvious some ways to organize existing infra

[rsimha]

What if we wrote build-system in TypeScript?

I agree with the long-term goal of moving to TS. However, I suspect it will be a multi-quarter effort to rewrite the 40K+ lines of JS code in build-system/. Not sure if / when we'll have the bandwidth to prioritize something like this. (Or do you think I'm overestimating the difficulty of this job? 😃)

Meanwhile, I think it's worth enforcing and backfilling JSDoc annotations, which should take substantially less time than a full rewrite, and help avoid simple coding bugs.

Also, the link I shared in the issue description seems to suggest that it's possible to directly type-check JSDoc in .js files by invoking typescript with the --checkJs flag. If this actually works, it too will be substantially easier than a full rewrite.

[samouri]

I agree with the long-term goal of moving to TS. However, I suspect it will be a multi-quarter effort to rewrite the 40K+ lines of JS code in build-system/. Not sure if / when we'll have the bandwidth to prioritize something like this. (Or do you think I'm overestimating the difficulty of this job? 😃)

I don't think a full rewrite is needed before we can start using TS, we can adopt it incrementally. Some possible ways include:

1. Converting files to .ts piecemeal instead of all at once. This allows us to have a relatively more strict tsconfig.
2. Converting them all at once but having an initially lax tsconfig that we gradually make more strict (e.g. with any's).

[rsimha]

Dusting off this old issue and assigning to our new Infra friend @rileyajones, who mentioned wanting to do something like this today 😃

[rcebulko]

I'll echo @samouri above and +1 the suggestion to make all of build-system run TypeScript all at once, but just spit out warnings until tasks can gradually be migrated

[jridgewell]

Converting files to .ts piecemeal instead of all at once. This allows us to have a relatively more strict tsconfig.

I would instead suggest using JS syntax only with Typescript JSDoc annotations. 0 runtime dependencies to run the code, which means running gulp (which is already extremely slow for us) won't get any slower.

[rsimha]

With #34544, we have started type-checking build-system/ during CI. The syntax remains mostly JS (easy to write and maintain build code), but type-checking is done via the TypeScript engine (ensuring code correctness and safety). Nice work @rileyajones!