feat: migrate lib to typescript #40

lokshunhung · 2020-10-28T08:01:45Z

This pr addresses the following goals in #39

Bump babel & related dependencies to latest, add @babel/preset-typescript
@babel/* dependencies are updated to the latest 7.12.x
Add tsconfig.json with strict type-checking enabled
tsconfig.json is initialized with tsc --init with a few more options enabled
Add typecheck script to package.json#scripts
Type checking of the lib directory can be execute with yarn run typecheck
Migrate files in lib one-by-one to typescript & make necessary changes for type-checking to pass
All files in lib are migrated to typescript with the following changes that might be potentially breaking for downstream packages:
- toTestResult.ts return type has some properties changed from null to undefined, and some more properties are added
- fail.ts additional error message handling (I'm not sure if the original behaviour of selectively passing only options.errorMessage but not options.test.errorMessage to the argument of toTestResult call is intentional, but it would be great to have it clarified / deprecate one of the options of passing error message in a future PR)
- More details can be found in the individual commit messages :)

lokshunhung · 2020-10-28T08:24:58Z

ESLint is failing due to missing resolvers from eslint-plugin-import

I'll add the missing eslint settings and the required peer dependency @typescript-eslint/parser

.babelrc

.yarnrc.yml

package.json

tsconfig.json

package.json

lokshunhung · 2020-10-28T09:31:36Z

ESLint is failing due to missing resolvers from eslint-plugin-import

I'll add the missing eslint settings and the required peer dependency @typescript-eslint/parser

Ok funny thing: it is failing not due to eslint-plugin-imports settings, but rather babel excludes ts files during compilation by default. So no files are present at the build/ directory when the integration test runs.

I'll add the missing -x ".ts" flag to the babel scripts😅

SimenB · 2020-10-28T09:51:42Z

I'll add the missing -x ".ts" flag to the babel scripts😅

please use the full --extensions flag 🙂

lokshunhung · 2020-10-28T11:04:00Z

Node 10 is failing due to unexpected token at _globalConfig and _context

class BaseTestRunner {
  _globalConfig;
  _context;
  constructor(globalConfig, context) {
    this._globalConfig = globalConfig;
    this._context = context;
  }
}

I'll add the declare keyword so the lines that declare class field do not emit code.

lib/createJestRunner.ts

SimenB · 2020-11-02T15:23:31Z

I've landed the eslint changes in #45 to reduce the diff here - could you rebase? Also make sure to keep the fix from #43 🙂

lokshunhung · 2020-11-03T02:45:52Z

I've landed the eslint changes in #45 to reduce the diff here - could you rebase? Also make sure to keep the fix from #43 🙂

I rebased and fix some lint errors. Unfortunately some lint rules have to be disabled:

import/export: false positive about duplicate default export with ts function overloading syntax
no-useless-constructor: throws an error when encountering ts ambient class declarations

~~I kept the fix but it now looks kindda ugly with the type-cast:~~

errorMessage || test.errorMessage
  ? [(errorMessage || test.errorMessage) as string]
  : []

Edit: toTestResult.ts is refactored to follow fail.ts so it's ok now

SimenB

can you remove

create-jest-runner/.eslintignore

Lines 4 to 5 in 3f50f78

    
           # remove after migrating to TS 
        
           types/

as well?

.eslintrc.json

lib/toTestResult.ts

package.json

SimenB

Thanks! I just left some nits, but overall this is really solid 👍

SimenB · 2020-11-04T15:27:15Z

lib/createJestRunner.ts

+    readonly _globalConfig: Config.GlobalConfig;
+
+    readonly _context: JestRunner.TestRunnerContext;


Suggested change

readonly _globalConfig: Config.GlobalConfig;

readonly _context: JestRunner.TestRunnerContext;

private readonly _globalConfig: Config.GlobalConfig;

private readonly _context: JestRunner.TestRunnerContext;

Making _globalConfig and _context private requires removing the corresponding class properties in the declare class TestRunner { /* */ } otherwise typescript complains about Types have separate declarations of a private property '_globalConfig'.

I can add the private keyword if you think this is good enough :)

lib/createJestRunner.ts

SimenB · 2020-11-04T15:28:21Z

lib/createJestRunner.ts

@@ -53,7 +77,7 @@ const createRunner = (runPath, { getExtraOptions } = {}) => {
                }

                return onStart(test).then(() => {
-                  // eslint-disable-next-line import/no-dynamic-require, global-require
+                  // eslint-disable-next-line import/no-dynamic-require, global-require, @typescript-eslint/no-var-requires
                  const runner = require(runPath);


I wonder if we should do import(runPath) here?

It can be done by changing it to:

onStart(test).then(() => import(runPath)).then(runner => { /* */ })

But babel would emit more code

onStart(test) .then(() => Promise.resolve(`${runPath}`) .then(s => _interopRequireWildcard(require(s)))) .then(runner => { /* */ })

Instead of what it currently emits:

onStart(test).then(() => { const runner = require(runPath); /* */ })

we should change when we drop node 10

package.json

SimenB · 2020-11-04T15:30:31Z

lib/types.ts

+}
+
+// Copied and adapted from https://github.com/facebook/jest/blob/2dafb09d51584d3785f3280f569784ec4334b5d8/packages/jest-runner/src/index.ts#L48-L285
+export declare abstract class TestRunner {


is this something we should export from Jest?

Actually it is exported from "jest-runner" as

declare class JestRunner { /* ... */ } export = JestRunner

But unfortunately I cannot use this JestRunner to annotate the return type of createTestRunner since it contains properties that our BaseTestRunner does not have, namely:

private readonly eventEmitter: Emittery.Typed<JestTestEvents>

readonly __PRIVATE_UNSTABLE_API_supportsEventEmitters__: boolean

readonly isSerial?: boolean

on: Emittery.Typed<JestTestEvents> (an alias of eventEmitter but bound to this.eventEmitter)

I actually dug into the jest codebase but it seems that the event emitter api seems to be considered unstable and prone to change in jest v27 (I suspect it has something to do with the jest-circus and flux-based architecture thingy), and I'm not sure if it is the right thing to add an additional dependency emittery to the project.

I couldn't find related docs or changelogs for the eventEmitters which were added since this commit so I decided I would copy and paste the JestRunner class declaration from jest-runner package with the fields that BaseTestRunner does not implement removed.

I tried and I can change TestRunner in lib/types.ts to

export declare abstract class TestRunner implements Omit<JestRunner, '__PRIVATE_UNSTABLE_API_supportsEventEmitters__' | 'on'> { constructor(globalConfig: JestConfig.GlobalConfig); runTests( tests: Array<JestRunner.Test>, watcher: JestRunner.TestWatcher, onStart: JestRunner.OnTestStart, onResult: JestRunner.OnTestSuccess, onFailure: JestRunner.OnTestFailure, options: JestRunner.TestRunnerOptions, ): Promise<void>; }

But I'm not sure if using the Omit<> utility type here is good practice. Any suggestions on improvements?

lib/types.ts

lib/toTestResult.ts

integrationTests/runner/run.js

.babelrc

lokshunhung · 2021-01-06T06:00:47Z

Hey @SimenB happy new year🙂 any chance that we could continue this in 2021?

SimenB · 2021-08-26T12:40:57Z

@lokshunhung happy new year! 😅 I've completely dropped the ball on this one... Would you be up for rebasing it? Feel free to squash everything into a single commit for that to avoid horrible conflicts repeatedly. After that I'll take a new look 🙂 (please ping me when you've rebased, GH doesn't always send an email for pushes)

chore: install @babel/preset-typescript ^7.0.0 chore: enable @babel/preset-typescript in babelrc enable options defaults to be enabled in babel 8 disable babelrc allowNamespaces because namespaces are legacy ts features and probably should not be used chore: add tsconfig.json esModuleInterop is disabled or else https://github.com/facebook/jest/blob/master/scripts/babel-plugin-jest-replace-ts-require-assignment.js might be needed to be copied to this repo Co-authored-by: Simen Bekkhus <[email protected]> enable emitDeclarationOnly in tsconfig.json Co-authored-by: Simen Bekkhus <[email protected]> chore: enable importsNotUsedAsValues, disable isloatedModules in tsconfig.json Co-authored-by: Simen Bekkhus <[email protected]> refactor: migrate toTestResult.js to ts refactor: fix toTestResult.ts types Allowing type checking to pass requires the following changes: - Remove optional result.console: null - Add result.leaks: false - Add result.openHandles: [] - Add result.perfStats.runtime: (end - start) - Add result.perfStats.slow: false - Add result.snapshot.uncheckedKeys: [] - Remove optional result.sourceMaps: {} - Remove optional result.testExecError: null - Add result.testResults[].failureDetails: [] - Change result.testResults[].failureMessage to always return a string - Change result.testResults[].fullName to always return a string refactor: extract & document getPerfStats fn in toTestResult.ts refactor: extract getSnapshot fn from toTestResult.ts refactor: extract getTestResults fn from toTestResult.ts refactor: migrate todo.js to ts refactor: extract common types from todo.ts refactor: migrate skip.js to ts refactor: extract common types from skip.ts refactor: migrate pass.js to ts refactor: extract common types from pass.ts refactor: migrate fail.js to ts fix: add fail.ts errorMessage to toTestResult options.tests refactor: migrate createJestRunner.js to ts refactor: _createParallelTestRun return Promise<void> in createJestRunner.ts The method _createParallelTestRun returns Promise<PollExitRun> implicitly via cleanup By wrapping the cleanup function with an arrow function body, it explicitly returns undefined (void) This allows _createParallelTestRun to remove the type cast in its return refactor: migrate index.js to ts chore: add eslint-plugin-import ts settings merge the settings from eslint-plugin-import/typescript preset with eslint-config-airbnb-base https://github.com/benmosher/eslint-plugin-import/blob/4f1101e584558d9c686144b71222acaaf4f70b72/config/typescript.js https://github.com/airbnb/javascript/blob/c5bee75b1b358a3749f1a6d38ee6fad73de28e29/packages/eslint-config-airbnb-base/rules/imports.js chore: use eslint plugin:import/typescript less boilerplate settings seems better than adding a bunch of settings chore: disable no-console lint warning in fixtures chore: disable import/no-unresolved lint for build artifacts fix: add --extensions '.ts' flag so babel compiles ts chore: update script to build types chore: add @babel/plugin-proposal-class-properties this fixes node 10 tests failing due to class properties declaration not being tranformed build: add yarn build-types to ci chore: disable no-useless-constructor lint this rule throws an error of `Cannot read property 'body' of null` when encountering ts ambient class declarations chore: update import/extensions lint options eslint-config-airbnb-base does not include .ts extension in this rule https://github.com/airbnb/javascript/blob/c5bee75b1b358a3749f1a6d38ee6fad73de28e29/packages/eslint-config-airbnb-base/rules/imports.js#L139-L143 chore: disable import/export lint this rule does not understand ts function overloading chore: remove tsc noEmit flag in package.json setting emitDeclarationOnly flag is sufficient for tsc to not emit js files Co-authored-by: Simen Bekkhus <[email protected]> chore: update watch script in package.json Co-authored-by: Simen Bekkhus <[email protected]> chore: remote duplicate eslint override for fixtures refactor: extract error message handling logic in toTestResult.ts chore: update lint rule overrides for js files refactor: fix explicit-module-boundary-types lint warnings chore: remove unused eslintignore glob chore: remove "types/" from files in package.json refactor: extract CreateRunnerOptions in createJestRunner.ts chore: remove unused 'typecheck' npm script it's basically the same as 'build-types' since the '--noEmit' flag was removed chore: remove unnecessary onlyRemoveTypeImports in .babelrc refactor: reuse jest types in types.ts Co-authored-by: Simen Bekkhus <[email protected]> chore: fix lint errors

lokshunhung · 2021-08-27T05:04:57Z

I squashed everything and rebased. Here are the list of changes I made in order to rebase:

Change how we import jest-worker to this
Added skipLibCheck in tsconfig, otherwise we type check our dependencies and get errors from @jest/console
Minor changes related to eslint (ignored warnings in test files here and here)

I appreciate if you could take a look @SimenB 😄

it is shipped by default by preset-env

SimenB · 2021-09-01T10:26:10Z

Thanks for working on this @lokshunhung!

SimenB · 2021-09-01T10:29:42Z

https://github.com/jest-community/create-jest-runner/releases/tag/v0.9.0

lokshunhung mentioned this pull request Oct 28, 2020

Rewrite in typescript #39

Closed

6 tasks

SimenB reviewed Oct 28, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

lokshunhung mentioned this pull request Oct 28, 2020

chore: bump dependencies in lockfile #42

Merged

SimenB reviewed Nov 1, 2020

View reviewed changes

lib/createJestRunner.ts Outdated Show resolved Hide resolved

lokshunhung force-pushed the use-typescript branch from 6e910e1 to e51967c Compare November 2, 2020 07:48

SimenB mentioned this pull request Nov 2, 2020

chore: add @typescript-eslint #45

Merged

lokshunhung force-pushed the use-typescript branch from 1ecb9f8 to a4eb7c4 Compare November 3, 2020 02:39

SimenB reviewed Nov 3, 2020

View reviewed changes

.eslintrc.json Outdated Show resolved Hide resolved

.eslintrc.json Outdated Show resolved Hide resolved

lib/toTestResult.ts Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

SimenB approved these changes Nov 4, 2020

View reviewed changes

SimenB requested review from jeysal, rogeliog and thymikee November 4, 2020 15:33

jeysal reviewed Nov 4, 2020

View reviewed changes

.babelrc Outdated Show resolved Hide resolved

lokshunhung force-pushed the use-typescript branch from 6b619d7 to 99ee3fd Compare November 5, 2020 10:28

lokshunhung added 2 commits August 27, 2021 12:47

fix: tsc skip checking node_modules types

b0b8d11

lokshunhung force-pushed the use-typescript branch from 99ee3fd to b0b8d11 Compare August 27, 2021 04:55

lokshunhung requested a review from SimenB August 27, 2021 05:05

SimenB added 2 commits August 27, 2021 09:54

chore: use node 14 types

372f8e3

add type dependencies as peer deps

d752ebe

SimenB added 3 commits September 1, 2021 11:38

Merge branch 'main' into use-typescript

37effda

minor cleanup

fa7ddc2

remove unused babel plugin

0d8f117

it is shipped by default by preset-env

SimenB approved these changes Sep 1, 2021

View reviewed changes

SimenB changed the title ~~Migrate lib to typescript~~ feat: migrate lib to typescript Sep 1, 2021

SimenB merged commit c5a8369 into jest-community:main Sep 1, 2021

lokshunhung deleted the use-typescript branch October 29, 2021 04:55

		readonly _globalConfig: Config.GlobalConfig;

		readonly _context: JestRunner.TestRunnerContext;

feat: migrate lib to typescript #40

feat: migrate lib to typescript #40

Uh oh!

Conversation

lokshunhung commented Oct 28, 2020

Uh oh!

lokshunhung commented Oct 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lokshunhung commented Oct 28, 2020

Uh oh!

SimenB commented Oct 28, 2020

Uh oh!

lokshunhung commented Oct 28, 2020

Uh oh!

Uh oh!

SimenB commented Nov 2, 2020

Uh oh!

lokshunhung commented Nov 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SimenB left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SimenB left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lokshunhung commented Jan 6, 2021

Uh oh!

SimenB commented Aug 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lokshunhung commented Aug 27, 2021

Uh oh!

SimenB commented Sep 1, 2021

Uh oh!

SimenB commented Sep 1, 2021

Uh oh!

Uh oh!

lokshunhung commented Oct 28, 2020 •

edited

Loading

lokshunhung commented Nov 3, 2020 •

edited

Loading

SimenB commented Aug 26, 2021 •

edited

Loading