From 9d63667c6dfc1aae96142d31f8fa6899cd24109a Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Tue, 5 Nov 2024 09:41:01 -0500 Subject: [PATCH 01/12] add ci --- .github/workflows/typescript-check.yml | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 .github/workflows/typescript-check.yml diff --git a/.github/workflows/typescript-check.yml b/.github/workflows/typescript-check.yml new file mode 100644 index 000000000..ead2b45a8 --- /dev/null +++ b/.github/workflows/typescript-check.yml @@ -0,0 +1,25 @@ +name: Pre-commit for code style and formatting checks + +on: + push: + branches: + - main + - 'release/**' + - 'feature/**' + - 618-typescript-ci + pull_request: + branches: + - main + - 'release/**' + - 'feature/**' + +jobs: + typescript-check: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install dependencies + run: npm ci + - name: Check TypeScript types + run: npx tsc -p . --emitDeclarationOnly false --noEmit + From ef203191e340bc2eaacb371c0efe385135b58af0 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Tue, 5 Nov 2024 11:55:38 -0500 Subject: [PATCH 02/12] test --- .github/workflows/typescript-check.yml | 4 +-- tools/check_typescipt_ci.py | 34 ++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 tools/check_typescipt_ci.py diff --git a/.github/workflows/typescript-check.yml b/.github/workflows/typescript-check.yml index ead2b45a8..f1fba4d6a 100644 --- a/.github/workflows/typescript-check.yml +++ b/.github/workflows/typescript-check.yml @@ -21,5 +21,5 @@ jobs: - name: Install dependencies run: npm ci - name: Check TypeScript types - run: npx tsc -p . --emitDeclarationOnly false --noEmit - + run: python tools/check_typescript_ci.py + diff --git a/tools/check_typescipt_ci.py b/tools/check_typescipt_ci.py new file mode 100644 index 000000000..09825d565 --- /dev/null +++ b/tools/check_typescipt_ci.py @@ -0,0 +1,34 @@ +from subprocess import run +from re import fullmatch + + +def main(): + print("Checking TypeScript types...") + res = run( + ["npx", "tsc", "-p", ".", "--emitDeclarationOnly", "false", "--noEmit"], + capture_output=True, + ) + + if res.returncode == 0: + return + + errors = [] + for line in res.stdout.decode("utf-8").splitlines(): + if len(line) == 0: + continue + if line[0] == " " and len(errors) > 0: + errors[-1] += "\n" + line + else: + errors.append(line) + + for err in errors: + match = fullmatch(r"(.+?)\((\d+),(\d+)\): ([\s\S]+)", err) + if match is None: + continue + + file, line, col, msg = match.groups() + print(f"::error file={file},line={line},col={col}::{msg}") + + +if __name__ == "__main__": + main() From 7290a89fcd35460a21d0118406c5993514e1be96 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Tue, 5 Nov 2024 11:58:00 -0500 Subject: [PATCH 03/12] test --- tools/{check_typescipt_ci.py => check_typescript_ci.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tools/{check_typescipt_ci.py => check_typescript_ci.py} (100%) diff --git a/tools/check_typescipt_ci.py b/tools/check_typescript_ci.py similarity index 100% rename from tools/check_typescipt_ci.py rename to tools/check_typescript_ci.py From e029fe7a5bd9a71f662c8a2f8208d0ff982450d9 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Tue, 5 Nov 2024 12:02:05 -0500 Subject: [PATCH 04/12] test --- .../src/js/src/DashboardPlugin/DashboardPlugin.tsx | 2 +- tools/check_typescript_ci.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/dashboard-object-viewer/src/js/src/DashboardPlugin/DashboardPlugin.tsx b/plugins/dashboard-object-viewer/src/js/src/DashboardPlugin/DashboardPlugin.tsx index 75e6c4cad..1f220d635 100644 --- a/plugins/dashboard-object-viewer/src/js/src/DashboardPlugin/DashboardPlugin.tsx +++ b/plugins/dashboard-object-viewer/src/js/src/DashboardPlugin/DashboardPlugin.tsx @@ -28,7 +28,7 @@ export function DashboardPlugin({ widget: VariableDefinition; }) => { const { id: widgetId, name, type } = widget; - if (type === dh.VariableType.TABLE || type === dh.VariableType.FIGURE) { + if (type === "dh.VariableType.TABLE" || type === dh.VariableType.FIGURE) { // Just ignore table and figure types - only want interesting other types return; } diff --git a/tools/check_typescript_ci.py b/tools/check_typescript_ci.py index 09825d565..72b8c05d2 100644 --- a/tools/check_typescript_ci.py +++ b/tools/check_typescript_ci.py @@ -27,7 +27,9 @@ def main(): continue file, line, col, msg = match.groups() - print(f"::error file={file},line={line},col={col}::{msg}") + print(f"::error file={file},line={line},col={col}::{msg.replace('\n', '%0A')}") + + raise Exception("TypeScript type checking failed") if __name__ == "__main__": From 1129d9abd74c1167c18426c184ab8b5476edab18 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Tue, 5 Nov 2024 12:04:01 -0500 Subject: [PATCH 05/12] test --- tools/check_typescript_ci.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/check_typescript_ci.py b/tools/check_typescript_ci.py index 72b8c05d2..0677d418a 100644 --- a/tools/check_typescript_ci.py +++ b/tools/check_typescript_ci.py @@ -27,7 +27,8 @@ def main(): continue file, line, col, msg = match.groups() - print(f"::error file={file},line={line},col={col}::{msg.replace('\n', '%0A')}") + msg = msg.replace("\n", "%0A") + print(f"::error file={file},line={line},col={col}::{msg}") raise Exception("TypeScript type checking failed") From 188562c22c3f2962c15a28dbe3632f96b839e890 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Tue, 5 Nov 2024 12:16:58 -0500 Subject: [PATCH 06/12] add --- .../src/js/src/DashboardPlugin/DashboardPlugin.tsx | 5 ++++- plugins/ui/src/js/src/elements/UITable/UITable.tsx | 4 +++- tools/check_typescript_ci.py | 9 +++++---- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/plugins/dashboard-object-viewer/src/js/src/DashboardPlugin/DashboardPlugin.tsx b/plugins/dashboard-object-viewer/src/js/src/DashboardPlugin/DashboardPlugin.tsx index 1f220d635..b150ac4df 100644 --- a/plugins/dashboard-object-viewer/src/js/src/DashboardPlugin/DashboardPlugin.tsx +++ b/plugins/dashboard-object-viewer/src/js/src/DashboardPlugin/DashboardPlugin.tsx @@ -28,7 +28,10 @@ export function DashboardPlugin({ widget: VariableDefinition; }) => { const { id: widgetId, name, type } = widget; - if (type === "dh.VariableType.TABLE" || type === dh.VariableType.FIGURE) { + if ( + type === 'dh.VariableType.TABLE' || + type === 'dh.VariableType.FIGURE' + ) { // Just ignore table and figure types - only want interesting other types return; } diff --git a/plugins/ui/src/js/src/elements/UITable/UITable.tsx b/plugins/ui/src/js/src/elements/UITable/UITable.tsx index 6ebb6ea9f..a3e4426e6 100644 --- a/plugins/ui/src/js/src/elements/UITable/UITable.tsx +++ b/plugins/ui/src/js/src/elements/UITable/UITable.tsx @@ -258,7 +258,9 @@ export function UITable({ if (alwaysFetchColumnsArray[0] === false) { return []; } - return alwaysFetchColumnsArray.filter(v => typeof v === 'string'); + return alwaysFetchColumnsArray.filter( + v => typeof v === 'string' + ) as string[]; }, [alwaysFetchColumnsArray, modelColumns]); const mouseHandlers = useMemo( diff --git a/tools/check_typescript_ci.py b/tools/check_typescript_ci.py index 0677d418a..9ead5a8ac 100644 --- a/tools/check_typescript_ci.py +++ b/tools/check_typescript_ci.py @@ -1,5 +1,6 @@ -from subprocess import run from re import fullmatch +from subprocess import run +from sys import exit def main(): @@ -10,7 +11,7 @@ def main(): ) if res.returncode == 0: - return + return 0 errors = [] for line in res.stdout.decode("utf-8").splitlines(): @@ -30,8 +31,8 @@ def main(): msg = msg.replace("\n", "%0A") print(f"::error file={file},line={line},col={col}::{msg}") - raise Exception("TypeScript type checking failed") + return res.returncode if __name__ == "__main__": - main() + exit(main()) From 726b8e432b19c20b4a5a6c49507911dd47275682 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Tue, 5 Nov 2024 12:17:56 -0500 Subject: [PATCH 07/12] update name --- .github/workflows/typescript-check.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/typescript-check.yml b/.github/workflows/typescript-check.yml index f1fba4d6a..ed3a08dda 100644 --- a/.github/workflows/typescript-check.yml +++ b/.github/workflows/typescript-check.yml @@ -1,4 +1,4 @@ -name: Pre-commit for code style and formatting checks +name: Check TypeScript types on: push: From 57a9a1a3d5e0519faa376350044140d219160ad5 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Tue, 5 Nov 2024 12:20:56 -0500 Subject: [PATCH 08/12] update ci --- .github/workflows/typescript-check.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/typescript-check.yml b/.github/workflows/typescript-check.yml index ed3a08dda..73042926a 100644 --- a/.github/workflows/typescript-check.yml +++ b/.github/workflows/typescript-check.yml @@ -6,7 +6,6 @@ on: - main - 'release/**' - 'feature/**' - - 618-typescript-ci pull_request: branches: - main @@ -16,6 +15,9 @@ on: jobs: typescript-check: runs-on: ubuntu-latest + concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true steps: - uses: actions/checkout@v4 - name: Install dependencies From 3f79270439599c1b51f35d30ca5474882d83df74 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Thu, 5 Dec 2024 12:44:41 -0500 Subject: [PATCH 09/12] Update plugins/dashboard-object-viewer/src/js/src/DashboardPlugin/DashboardPlugin.tsx Co-authored-by: Mike Bender --- .../src/js/src/DashboardPlugin/DashboardPlugin.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/plugins/dashboard-object-viewer/src/js/src/DashboardPlugin/DashboardPlugin.tsx b/plugins/dashboard-object-viewer/src/js/src/DashboardPlugin/DashboardPlugin.tsx index b150ac4df..2475c1fb5 100644 --- a/plugins/dashboard-object-viewer/src/js/src/DashboardPlugin/DashboardPlugin.tsx +++ b/plugins/dashboard-object-viewer/src/js/src/DashboardPlugin/DashboardPlugin.tsx @@ -28,10 +28,7 @@ export function DashboardPlugin({ widget: VariableDefinition; }) => { const { id: widgetId, name, type } = widget; - if ( - type === 'dh.VariableType.TABLE' || - type === 'dh.VariableType.FIGURE' - ) { + if (type === 'Table' || type === 'Figure') { // Just ignore table and figure types - only want interesting other types return; } From d31033c12d99976c5cf5ecbf06702ffdfe7c39cd Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Tue, 10 Dec 2024 16:23:01 -0500 Subject: [PATCH 10/12] fix typing --- plugins/ui/src/js/src/elements/SearchField.tsx | 3 ++- .../src/js/src/elements/UITable/UITableModel.test.ts | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/plugins/ui/src/js/src/elements/SearchField.tsx b/plugins/ui/src/js/src/elements/SearchField.tsx index 4f58dd796..a58b292ce 100644 --- a/plugins/ui/src/js/src/elements/SearchField.tsx +++ b/plugins/ui/src/js/src/elements/SearchField.tsx @@ -8,9 +8,10 @@ import { useKeyboardEventCallback, } from './hooks'; import useDebouncedOnChange from './hooks/useDebouncedOnChange'; +import { SerializedTextInputEventProps } from './model'; export function SearchField( - props: DHCSearchFieldProps & { + props: SerializedTextInputEventProps & { onSubmit?: (value: string) => void; onClear?: () => void; } diff --git a/plugins/ui/src/js/src/elements/UITable/UITableModel.test.ts b/plugins/ui/src/js/src/elements/UITable/UITableModel.test.ts index 0c7df094e..707086e1b 100644 --- a/plugins/ui/src/js/src/elements/UITable/UITableModel.test.ts +++ b/plugins/ui/src/js/src/elements/UITable/UITableModel.test.ts @@ -33,6 +33,7 @@ describe('Formatting', () => { table: MOCK_TABLE, databars: [], format: [{ color: 'red' }, { color: 'blue' }], + displayNameMap: {}, }); expect(model.getFormatOptionForCell(0, 0, 'color')).toBe('blue'); expect(model.getFormatOptionForCell(1, 1, 'color')).toBe('blue'); @@ -48,6 +49,7 @@ describe('Formatting', () => { { cols: 'column0', color: 'red' }, { cols: 'column1', color: 'blue' }, ], + displayNameMap: {}, }); expect(model.getFormatOptionForCell(0, 0, 'color')).toBe('red'); expect(model.getFormatOptionForCell(1, 1, 'color')).toBe('blue'); @@ -71,6 +73,7 @@ describe('Formatting', () => { { color: 'red', if_: 'even' }, { cols: 'column1', color: 'blue', if_: 'even' }, ], + displayNameMap: {}, }); expect(model.getFormatOptionForCell(0, 0, 'color')).toBe('red'); expect(model.getFormatOptionForCell(0, 1, 'color')).toBeUndefined(); @@ -86,6 +89,7 @@ describe('Formatting', () => { table: MOCK_TABLE, databars: [], format: [{ cols: 'column0', color: 'red' }], + displayNameMap: {}, }); expect(model.getFormatOptionForCell(1, 1, 'color')).toBeUndefined(); expect( @@ -104,6 +108,7 @@ describe('Formatting', () => { table: MOCK_TABLE, databars: [], format: [{ color: 'red', if_: 'even' }], + displayNameMap: {}, }); expect(model.getFormatOptionForCell(0, 0, 'color')).toBeUndefined(); expect(model.getFormatOptionForCell(0, 1, 'color')).toBeUndefined(); @@ -119,6 +124,7 @@ describe('Formatting', () => { table: MOCK_TABLE, databars: [], format: [{ color: 'red' }], + displayNameMap: {}, }); expect(model.colorForCell(0, 0, {} as IrisGridThemeType)).toBe('red'); }); @@ -130,6 +136,7 @@ describe('Formatting', () => { table: MOCK_TABLE, databars: [], format: [], + displayNameMap: {}, }); expect(model.colorForCell(0, 0, {} as IrisGridThemeType)).toBeUndefined(); expect(MOCK_BASE_MODEL.colorForCell).toHaveBeenCalledTimes(1); @@ -145,6 +152,7 @@ describe('Formatting', () => { table: MOCK_TABLE, databars: [], format: [{ background_color: 'black' }], + displayNameMap: {}, }); expect( model.colorForCell(0, 0, { white: 'white' } as IrisGridThemeType) @@ -162,6 +170,7 @@ describe('Formatting', () => { table: MOCK_TABLE, databars: [], format: [{ background_color: 'white' }], + displayNameMap: {}, }); expect( model.colorForCell(0, 0, { black: 'black' } as IrisGridThemeType) @@ -176,6 +185,7 @@ describe('Formatting', () => { table: MOCK_TABLE, databars: [], format: [{ color: 'foo' }], + displayNameMap: {}, }); model.setColorMap(new Map([['foo', 'bar']])); expect(model.colorForCell(0, 0, {} as IrisGridThemeType)).toBe('bar'); @@ -190,6 +200,7 @@ describe('Formatting', () => { table: MOCK_TABLE, databars: [], format: [], + displayNameMap: {}, }); expect( model.backgroundColorForCell(0, 0, {} as IrisGridThemeType) @@ -204,6 +215,7 @@ describe('Formatting', () => { table: MOCK_TABLE, databars: [], format: [{ background_color: 'foo' }], + displayNameMap: {}, }); model.setColorMap(new Map([['foo', 'bar']])); expect(model.backgroundColorForCell(0, 0, {} as IrisGridThemeType)).toBe( From 167606c912bcab5a6059d2f3f361f03b9b7666d8 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Mon, 16 Dec 2024 17:11:05 -0500 Subject: [PATCH 11/12] add comments --- tools/check_typescript_ci.py | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/tools/check_typescript_ci.py b/tools/check_typescript_ci.py index 9ead5a8ac..5cefe6d56 100644 --- a/tools/check_typescript_ci.py +++ b/tools/check_typescript_ci.py @@ -13,26 +13,38 @@ def main(): if res.returncode == 0: return 0 - errors = [] + messages = [] for line in res.stdout.decode("utf-8").splitlines(): if len(line) == 0: continue - if line[0] == " " and len(errors) > 0: - errors[-1] += "\n" + line + # If there's an indent, that means it's a continuation of the previous line + # For example, the error message could be like: + # > Argument of type 'FUNCTION_1_TYPE | undefined' is not assignable to parameter of type 'FUNCTION_2_TYPE'. + # > Type 'FUNCTION_1_TYPE' is not assignable to type 'FUNCTION_2_TYPE'. + # > Types of parameters `PARAM_1` and `PARAM_2` are incompatible. + # > Type 'PARAM_1_TYPE' is not assignable to type 'PARAM_2_TYPE'. + if line[0] == " " and len(messages) > 0: + messages[-1] += "\n" + line else: - errors.append(line) + messages.append(line) - for err in errors: - match = fullmatch(r"(.+?)\((\d+),(\d+)\): ([\s\S]+)", err) + for message in messages: + # Check if the message is actually and extract the details + # Error message format: file(line,col): error_message + match = fullmatch(r"(.+?)\((\d+),(\d+)\): ([\s\S]+)", message) if match is None: continue - file, line, col, msg = match.groups() - msg = msg.replace("\n", "%0A") - print(f"::error file={file},line={line},col={col}::{msg}") + file, line, col, error_message = match.groups() + # Newlines in GitHub Actions annotations are escaped as %0A + # https://github.com/actions/toolkit/issues/193#issuecomment-605394935 + error_message = error_message.replace("\n", "%0A") + # GitHub Actions annotation format + print(f"::error file={file},line={line},col={col}::{error_message}") return res.returncode if __name__ == "__main__": + # Exit with returncode so GitHub Actions fails properly exit(main()) From f751e0712ecf2ccd917549e4bb3f68e53aaac161 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Tue, 17 Dec 2024 17:06:31 -0500 Subject: [PATCH 12/12] Update tools/check_typescript_ci.py Co-authored-by: Mike Bender --- tools/check_typescript_ci.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/check_typescript_ci.py b/tools/check_typescript_ci.py index 5cefe6d56..6f07f38dd 100644 --- a/tools/check_typescript_ci.py +++ b/tools/check_typescript_ci.py @@ -29,7 +29,7 @@ def main(): messages.append(line) for message in messages: - # Check if the message is actually and extract the details + # Check if the message is actually an error and extract the details # Error message format: file(line,col): error_message match = fullmatch(r"(.+?)\((\d+),(\d+)\): ([\s\S]+)", message) if match is None: