Skip to content

Commit

Permalink
add type checking (#1088)
Browse files Browse the repository at this point in the history
- fixes #1085
- include core, extensions, platform-bible-utils, platform-bible-react
- fix existing type checking errors
- add typecheck to pre-commit
- use `noImplicitReturns` instead of `consistent-return`
- type check in CI
- fix jsdom type check errors
- also make VS Code use the repo TS version
- remember to remove the patch and the type overrides when the BNF types are fixed
  • Loading branch information
irahopkinson authored Nov 11, 2024
1 parent 4221b37 commit ef5a034
Show file tree
Hide file tree
Showing 46 changed files with 175 additions and 337 deletions.
2 changes: 2 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ module.exports = {

// #region ERB rules

// Use `noImplicitReturns` instead. See https://typescript-eslint.io/rules/consistent-return/.
'consistent-return': 'off',
'import/extensions': 'off',
// A temporary hack related to IDE not resolving correct package.json
'import/no-extraneous-dependencies': 'off',
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ jobs:
- name: Install packages
run: npm ci

- name: type checking
run: npm run typecheck

- name: Build
run: npm run build

Expand Down
7 changes: 5 additions & 2 deletions .husky/pre-commit
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

echo "Running Lint check..."

echo "Lint checking..."
# generate papi.d.ts as part of lint:staged
npm run lint:staged
echo "Lint check finished"

echo "Type checking..."
npm run typecheck
echo "Type check finished"

echo "If the following fails run npm run editor:unlink"
npx yalc check
1 change: 1 addition & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"javascript.validate.enable": false,
"javascript.format.enable": false,
"typescript.format.enable": false,
"typescript.tsdk": "node_modules/typescript/lib",

"search.exclude": {
".git": true,
Expand Down
2 changes: 2 additions & 0 deletions extensions/.eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ module.exports = {

// #region ERB rules

// Use `noImplicitReturns` instead. See https://typescript-eslint.io/rules/consistent-return/.
'consistent-return': 'off',
'import/extensions': 'off',
// A temporary hack related to IDE not resolving correct package.json
'import/no-extraneous-dependencies': 'off',
Expand Down
1 change: 1 addition & 0 deletions extensions/src/c-sharp-provider-test/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
declare module 'c-sharp-provider-test' {
// @ts-ignore: TS2307 - Cannot find module '@papi/core' or its corresponding type declarations
import { IDataProvider, DataProviderDataType } from '@papi/core';

type TimeDataTypes = {
Expand Down
2 changes: 2 additions & 0 deletions extensions/src/hello-someone/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ module.exports = {

// #region ERB rules

// Use `noImplicitReturns` instead. See https://typescript-eslint.io/rules/consistent-return/.
'consistent-return': 'off',
'import/extensions': 'off',
// A temporary hack related to IDE not resolving correct package.json
'import/no-extraneous-dependencies': 'off',
Expand Down
3 changes: 2 additions & 1 deletion extensions/src/hello-someone/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
"lint:scripts": "eslint --ext .cjs,.js,.jsx,.ts,.tsx --cache .",
"lint:styles": "stylelint **/*.{css,scss}",
"lint-fix": "npm run lint-fix:scripts && npm run lint:styles -- --fix",
"lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts"
"lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts",
"typecheck": "tsc -p ./tsconfig.json"
},
"browserslist": [],
"peerDependencies": {
Expand Down
1 change: 1 addition & 0 deletions extensions/src/hello-someone/src/types/hello-someone.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
declare module 'hello-someone' {
// @ts-ignore: TS2307 - Cannot find module '@papi/core' or its corresponding type declarations
import { IDataProvider, DataProviderDataType } from '@papi/core';

export type Person = {
Expand Down
2 changes: 2 additions & 0 deletions extensions/src/hello-world/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ module.exports = {

// #region ERB rules

// Use `noImplicitReturns` instead. See https://typescript-eslint.io/rules/consistent-return/.
'consistent-return': 'off',
'import/extensions': 'off',
// A temporary hack related to IDE not resolving correct package.json
'import/no-extraneous-dependencies': 'off',
Expand Down
3 changes: 2 additions & 1 deletion extensions/src/hello-world/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
"lint:scripts": "eslint --ext .cjs,.js,.jsx,.ts,.tsx --cache .",
"lint:styles": "stylelint **/*.{css,scss}",
"lint-fix": "npm run lint-fix:scripts && npm run lint:styles -- --fix",
"lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts"
"lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts",
"typecheck": "tsc -p ./tsconfig.json"
},
"browserslist": [],
"peerDependencies": {
Expand Down
1 change: 1 addition & 0 deletions extensions/src/hello-world/src/types/hello-world.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
declare module 'hello-world' {
// @ts-ignore: TS2307 - Cannot find module '@papi/core' or its corresponding type declarations
import type { DataProviderDataType, MandatoryProjectDataTypes } from '@papi/core';
import type { IBaseProjectDataProvider } from 'papi-shared-types';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { VerseRef } from '@sillsdev/scripture';
import { VerseRef } from '@sillsdev/scripture';
import papi, { logger } from '@papi/frontend';
import {
useData,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ declare module 'legacy-comment-manager' {
DataProviderDataType,
DataProviderSubscriberOptions,
DataProviderUpdateInstructions,
// @ts-ignore: TS2307 - Cannot find module '@papi/core' or its corresponding type declarations
} from '@papi/core';
import type { IProjectDataProvider } from 'papi-shared-types';
import { UnsubscriberAsync } from 'platform-bible-utils';
Expand Down
2 changes: 2 additions & 0 deletions extensions/src/platform-scripture-editor/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ module.exports = {

// #region ERB rules

// Use `noImplicitReturns` instead. See https://typescript-eslint.io/rules/consistent-return/.
'consistent-return': 'off',
'import/extensions': 'off',
// A temporary hack related to IDE not resolving correct package.json
'import/no-extraneous-dependencies': 'off',
Expand Down
3 changes: 2 additions & 1 deletion extensions/src/platform-scripture-editor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
"lint:scripts": "eslint --ext .cjs,.js,.jsx,.ts,.tsx --cache .",
"lint:styles": "stylelint **/*.{css,scss}",
"lint-fix": "npm run lint-fix:scripts && npm run lint:styles -- --fix",
"lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts"
"lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts",
"typecheck": "tsc -p ./tsconfig.json"
},
"browserslist": [],
"peerDependencies": {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/**
* Work-around until the `@biblionexus-foundation/platform-editor` package fully includes its types.
* Note there is also a patch that should be removed when this is fixed.
*/

declare module 'shared-react/annotation/selection.model' {
type UsjLocation = {
jsonPath: string;
offset: number;
};

export type SelectionRange = {
start: UsjLocation;
end?: UsjLocation;
};

export type AnnotationRange = {
start: UsjLocation;
end: UsjLocation;
};
}

declare module 'shared-react/nodes/scripture/usj/ImmutableNoteCallerNode' {
export const immutableNoteCallerNodeName = 'ImmutableNoteCallerNode';
}

declare module 'shared-react/plugins/logger-basic.model' {
export type LoggerBasic = {
error(message: string): void;
warn(message: string): void;
info(message: string): void;
};
}

declare module 'shared-react/plugins/text-direction.model' {
/** Left-to-right or Right-to-left or Automatically determined from the content. */
export type TextDirection = 'ltr' | 'rtl' | 'auto';
}

declare module 'shared-react/nodes/scripture/usj/usj-node-options.model' {
import { SyntheticEvent } from 'react';

/** Option properties to use with each node. */
type NodeOptions = { [nodeClassName: string]: { [prop: string]: unknown } | undefined };
type OnClick = (event: SyntheticEvent) => void;
const immutableNoteCallerNodeName = 'ImmutableNoteCallerNode';

export const MarkNodeName = 'MarkNode';

export type AddMissingComments = (usjCommentIds: string[]) => void;

/** Options for each editor node. */
export interface UsjNodeOptions extends NodeOptions {
[immutableNoteCallerNodeName]?: {
/** Possible note callers to use when caller is '+'. */
noteCallers?: string[];
/** Click handler method. */
onClick?: OnClick;
};
[MarkNodeName]?: {
/** Method to add missing comments. */
addMissingComments?: AddMissingComments;
};
}
}
2 changes: 2 additions & 0 deletions extensions/src/platform-scripture/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ module.exports = {

// #region ERB rules

// Use `noImplicitReturns` instead. See https://typescript-eslint.io/rules/consistent-return/.
'consistent-return': 'off',
'import/extensions': 'off',
// A temporary hack related to IDE not resolving correct package.json
'import/no-extraneous-dependencies': 'off',
Expand Down
3 changes: 2 additions & 1 deletion extensions/src/platform-scripture/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
"lint:scripts": "eslint --ext .cjs,.js,.jsx,.ts,.tsx --cache .",
"lint:styles": "stylelint **/*.{css,scss}",
"lint-fix": "npm run lint-fix:scripts && npm run lint:styles -- --fix",
"lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts"
"lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts",
"typecheck": "tsc -p ./tsconfig.json"
},
"browserslist": [],
"peerDependencies": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ declare module 'platform-scripture' {
DataProviderUpdateInstructions,
ExtensionDataScope,
IDataProvider,
// @ts-ignore: TS2307 - Cannot find module '@papi/core' or its corresponding type declarations
} from '@papi/core';
import type { IProjectDataProvider } from 'papi-shared-types';
import { Dispose, LocalizeKey, UnsubscriberAsync } from 'platform-bible-utils';
Expand Down
1 change: 1 addition & 0 deletions extensions/src/project-notes-data-provider/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type {
DataProviderDataType,
DataProviderSubscriberOptions,
IDataProvider,
// @ts-ignore: TS2307 - Cannot find module '@papi/core' or its corresponding type declarations
} from '@papi/core';
import { PlatformEvent, Unsubscriber } from 'platform-bible-utils';

Expand Down
2 changes: 2 additions & 0 deletions extensions/src/quick-verse/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ module.exports = {

// #region ERB rules

// Use `noImplicitReturns` instead. See https://typescript-eslint.io/rules/consistent-return/.
'consistent-return': 'off',
'import/extensions': 'off',
// A temporary hack related to IDE not resolving correct package.json
'import/no-extraneous-dependencies': 'off',
Expand Down
3 changes: 2 additions & 1 deletion extensions/src/quick-verse/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
"lint:scripts": "eslint --ext .cjs,.js,.jsx,.ts,.tsx --cache .",
"lint:styles": "stylelint **/*.{css,scss}",
"lint-fix": "npm run lint-fix:scripts && npm run lint:styles -- --fix",
"lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts"
"lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts",
"typecheck": "tsc -p ./tsconfig.json"
},
"browserslist": [],
"peerDependencies": {
Expand Down
1 change: 1 addition & 0 deletions extensions/src/quick-verse/src/types/quick-verse.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
declare module 'quick-verse' {
// @ts-ignore: TS2307 - Cannot find module '@papi/core' or its corresponding type declarations
import { DataProviderDataType, IDataProvider } from '@papi/core';

export type QuickVerseSetData = string | { text: string; isHeresy: boolean };
Expand Down
2 changes: 1 addition & 1 deletion extensions/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"jsx": "react-jsx",
"typeRoots": [
// Include default type declarations
"./node_modules/@types",
"../node_modules/@types",
// Include papi-dts type declarations (for papi.d.ts)
"../lib",
// Include these bundled extensions' type declarations
Expand Down
3 changes: 2 additions & 1 deletion lib/papi-dts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@
"clean": "rimraf papi.tsbuildinfo",
"lint": "cross-env NODE_ENV=development eslint --ext .cjs,.js,.jsx,.ts,.tsx --cache .",
"lint-fix": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint",
"lint:staged": "lint-staged -q"
"lint:staged": "lint-staged -q",
"typecheck": "tsc -p ./tsconfig.lint.json"
},
"lint-staged": {
"*.{cjs,js,jsx,ts,tsx}": ["prettier --write", "cross-env NODE_ENV=development eslint --cache"],
Expand Down
10 changes: 8 additions & 2 deletions lib/papi-dts/tsconfig.lint.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
{
"extends": "./tsconfig",
"files": [".eslintrc.cjs", "edit-papi-d-ts.ts", "papi.d.ts"]
"extends": "../../tsconfig",
"compilerOptions": {
"module": "ESNext",
"baseUrl": "./",
"noEmit": true
},
"include": [".eslintrc.cjs", "edit-papi-d-ts.ts", "papi.d.ts"],
"exclude": []
}
3 changes: 2 additions & 1 deletion lib/platform-bible-react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
"lint:styles": "stylelint **/*.{css,scss}",
"lint-fix": "npm run lint-fix:scripts && npm run lint:styles -- --fix",
"lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts",
"test": "jest --silent"
"test": "jest --silent",
"typecheck": "tsc -p ./tsconfig.json"
},
"peerDependencies": {
"react": ">=18.3.1",
Expand Down
3 changes: 1 addition & 2 deletions lib/platform-bible-react/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
"extends": "../../tsconfig",
"compilerOptions": {
"baseUrl": "./src",
"rootDir": "./src",
"target": "ESNext",
"module": "ESNext",
// Running npm i removes packages that are in common between the current directory's
Expand All @@ -16,7 +15,7 @@
"incremental": false,
"isolatedModules": true,
"noEmit": true,
"noImplicitReturns": false,
"noImplicitReturns": true,
"skipLibCheck": false,
"useDefineForClassFields": true,
"outDir": "",
Expand Down
3 changes: 2 additions & 1 deletion lib/platform-bible-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@
"lint:scripts": "cross-env NODE_ENV=development eslint --ext .cjs,.js,.jsx,.ts,.tsx --cache .",
"lint-fix": "npm run lint-fix:scripts",
"lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts",
"test": "jest --silent"
"test": "jest --silent",
"typecheck": "tsc -p ./tsconfig.json"
},
"peerDependencies": {},
"dependencies": {
Expand Down
3 changes: 1 addition & 2 deletions lib/platform-bible-utils/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
"extends": "../../tsconfig",
"compilerOptions": {
"baseUrl": "./src",
"rootDir": "./src",
"target": "ESNext",
"module": "ESNext",
// Running npm i removes packages that are in common between the current directory's
Expand All @@ -16,7 +15,7 @@
"incremental": false,
"isolatedModules": true,
"noEmit": true,
"noImplicitReturns": false,
"noImplicitReturns": true,
"skipLibCheck": false,
"useDefineForClassFields": true,
"outDir": "",
Expand Down
4 changes: 4 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit ef5a034

Please sign in to comment.