Convert execute.js to typescript

[zepumph]

This central file will be a good sanity check for the typescript conversion project. This file is used everywhere, and will let us know quickly and loudly if we forgot about something over in phetsims/chipper#1481

[zepumph]

On hold until completed: #380

[zepumph]

@samreid and I feel unblocked to try to convert this now and see what goes wrong.

[zepumph]

We converted execute to typescript and immediately found two problems:

1. My webstorm external tools needed updating from "node". Here is what one looks like:
   
2. requiring execute.js fails because it is a ts file and js doesn't resolve dynamically to ts.

• I believe the best path forward is to undo all the .js suffixes we added to require statements, and instead assert that they don't have any extension.
• Then once a file is in typescript, it can be converted to use imports, and forced to add the .js.
• We could also go more "atomic" and attempt to convert many files to typescript. I don't recommend this.
• Paper trail for adding .js suffixes: require-statement-match problems chipper#1498

[zepumph]

I'm going to revert the .js extension additions over in the other issue.

[samreid]

• Better parametric type:

// Overload when options.errors is 'resolve'
export default function (
  cmd: string,
  args: string[],
  cwd: string,
  options: { errors: 'resolve'; [key: string]: any }
): Promise<{
  code: number;
  stdout: string;
  stderr: string;
  cwd: string;
  error?: Error;
  time: number;
}>;

// Overload when options.errors is 'reject' or undefined (default)
export default function (
  cmd: string,
  args: string[],
  cwd: string,
  options?: { errors?: 'reject'; [key: string]: any }
): Promise<string>;

// Implementation
export default function (
  cmd: string,
  args: string[],
  cwd: string,
  options?: any
): Promise<string | { code: number; stdout: string; stderr: string; cwd: string; error?: Error; time: number }> {
  // ...implementation
}

[samreid]

I decided to push changes from today to a new branch execute-typescript-perennial-403 in the following repos:

• aqua
• chipper
• perennial

I listed all commits above. Some of them also cross reference #410. Note the branch has the package.json ../../../ bug. Do not merge main into that feature branch unless you want about 50 merge conflicts. (But hopefully merging the feature branch into main will be ok, if I understand correctly.)

[zepumph]

Note the branch has the package.json ../../../ bug

We cherry picked that into the branch, instead of merging all of main into it, and it is working nicely.

[zepumph]

Ok. Merged to main and pushed. All that seems left is:

• TODOs
• monitor CT
• Better return type info based on resolve/reject errors.

[zepumph]

@marlitas reported that grunt check --all --clean was failing because phet-io is failing with this:

Running "check" task
../perennial/js/common/execute.ts:9:8 - error TS1192: Module '"child_process"' has no default export.

9 import child_process from 'child_process';
         ~~~~~~~~~~~~~

../perennial/js/common/execute.ts:10:21 - error TS2792: Cannot find module 'winston'. Did you mean to set the 'moduleResolution' option to 'nodenext', or to add aliases to the 'paths' option?

10 import winston from 'winston';
                       ~~~~~~~~~

../perennial/js/common/execute.ts:11:8 - error TS1259: Module '"C:/Users/mjkauzmann/PHET/git/perennial-alias/node_modules/@types/lodash/index"' can only be default-imported using the 'allowSyntheticDefaultImports' flag

11 import _ from 'lodash';
          ~

  ../perennial-alias/node_modules/@types/lodash/index.d.ts:14:1
    14 export = _;
       ~~~~~~~~~~~
    This module is declared with 'export =', and can only be used with a default import when using the 'allowSyntheticDefaultImports' flag.

../perennial/js/common/execute.ts:12:8 - error TS1259: Module '"assert"' can only be default-imported using the 'allowSyntheticDefaultImports' flag

12 import assert from 'assert';
          ~~~~~~

  ../perennial/node_modules/@types/node/assert.d.ts:1035:5
    1035     export = assert;
             ~~~~~~~~~~~~~~~~
    This module is declared with 'export =', and can only be used with a default import when using the 'allowSyntheticDefaultImports' flag.

../perennial/js/common/execute.ts:13:8 - error TS1259: Module '"grunt"' can only be default-imported using the 'allowSyntheticDefaultImports' flag

13 import grunt from 'grunt';
          ~~~~~

  ../perennial/node_modules/@types/grunt/index.d.ts:1331:5
    1331     export = grunt;
             ~~~~~~~~~~~~~~~
    This module is declared with 'export =', and can only be used with a default import when using the 'allowSyntheticDefaultImports' flag.

../perennial/js/common/execute.ts:63:31 - error TS7006: Parameter 'error' implicitly has an 'any' type.

63     childProcess.on( 'error', error => {
                                 ~~~~~

../perennial/js/common/execute.ts:75:37 - error TS7006: Parameter 'data' implicitly has an 'any' type.

75     childProcess.stderr.on( 'data', data => {
                                       ~~~~

../perennial/js/common/execute.ts:77:7 - error TS2578: Unused '@ts-expect-error' directive.

77       // @ts-expect-error
         ~~~~~~~~~~~~~~~~~~~

../perennial/js/common/execute.ts:81:37 - error TS7006: Parameter 'data' implicitly has an 'any' type.

81     childProcess.stdout.on( 'data', data => {
                                       ~~~~

../perennial/js/common/execute.ts:84:7 - error TS2578: Unused '@ts-expect-error' directive.

84       // @ts-expect-error
         ~~~~~~~~~~~~~~~~~~~

../perennial/js/common/execute.ts:89:31 - error TS7006: Parameter 'code' implicitly has an 'any' type.

89     childProcess.on( 'close', code => {
                                 ~~~~

[zepumph]

Ok. So type checking a js file that require()s a typescript file drops some type information I guess. Above I excluded these from being type checked, but I wonder if actually we should just have updated the tsconfig and linting config for phet-io to support node-based type checking for the scripts dir.

[zepumph]

I committed all changes for better typescript support, and got all TODOs handled. Is there anything more to do here? Please take a look at the above commit to phet-io/tsconfig.json during the review.

[samreid]

Everything looks great here. Nice work on the overloads in execute. I do not see a better alternative for now to the opt-out in phet-io/tsconfig.json. Closing.