CONSOLE-2501: Upgrade to Typescript v4 #10432

glekner · 2021-11-10T17:04:29Z

Updated Major typescript version to match the dynamic plugin sdk version 4.2.4
Updated ts-loader and ForkTsCheckerWebpackPlugin versions. cli logs about ts errors are now a lot clearer.
Updated i18n-parser to support ts4
Updated @typescript-eslint/eslint-plugin and @typescript-eslint/parser from v2 to v5
Updated eslint-plugin-react-hooks to v4 - Added lots of missing useMemos
Added globals like React JSX
Fixed breaking changes and rules
Changed Chainable<Subject> to Chainable (cypress). eslint would shout about unused vars, and since updating versions Subject is the default type interface Cypress.Chainable<Subject = any>

CLI Errors:
Before

After

glekner · 2021-11-10T17:04:45Z

please review @spadgett @vojtechszocs @rawagner @christianvogt

openshift-ci · 2021-11-10T17:07:07Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: glekner
To complete the pull request process, please assign invinciblejai after the PR has been reviewed.
You can assign the PR to them by writing /assign @invinciblejai in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

frontend/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

frontend/package.json

spadgett · 2021-11-10T17:41:20Z

...orage-plugin/src/components/create-storage-system/external-storage/ibm-flashsystem/index.tsx

@@ -166,7 +166,6 @@ export const createFlashSystemPayload: CreatePayload<FlashSystemState> = (
  const storageSecretTemplate: SecretKind = {
    apiVersion: apiVersionForModel(SecretModel),
    stringData: {
-      // eslint-disable-next-line @typescript-eslint/camelcase


Hm, I don't see why this comment could be removed. We want the camelcase rule active for most of the code. Did the defaults change?

frontend/packages/console-shared/src/components/popper/Popper.tsx

frontend/packages/dev-console/src/components/catalog/providers/useBuilderImages.tsx

spadgett · 2021-11-10T17:55:55Z

frontend/public/components/dashboard/dashboards-page/cluster-dashboard/inventory-card.tsx

@@ -79,6 +79,7 @@ const ClusterInventoryItem = withDashboardResources<ClusterInventoryItemProps>(
            });
      }, [mapperLoader]);

+      // eslint-disable-next-line react-hooks/exhaustive-deps


Why was this needed?

resolves
The 'additionalResourcesData' object makes the dependencies of useCallback Hook (at line 105) change on every render. To fix this, wrap the initialization of 'additionalResourcesData' in its own useMemo()

no need for useMemo here

That sounds like a legitimate problem that shouldn't be ignored?

spadgett · 2021-11-10T17:56:20Z

frontend/public/components/dashboard/dashboards-page/cluster-dashboard/utilization-card.tsx

@@ -1,3 +1,4 @@
+/* eslint-disable react-hooks/exhaustive-deps */


Let's avoid adding this at the top of the file. It's better to apply only where it's needed.

spadgett · 2021-11-10T17:56:28Z

frontend/public/components/dashboard/project-dashboard/utilization-card.tsx

@@ -1,3 +1,4 @@
+/* eslint-disable react-hooks/exhaustive-deps */


Same comment here.

spadgett · 2021-11-10T17:56:50Z

frontend/public/components/filter-toolbar.tsx

@@ -101,6 +101,7 @@ export const FilterToolbar: React.FC<FilterToolbarProps> = ({
  const labelFilterQueryArgumentKey = uniqueFilterName
    ? `${uniqueFilterName}-${labelFilter}`
    : labelFilter;
+  // eslint-disable-next-line react-hooks/exhaustive-deps


Why was this needed?

spadgett · 2021-11-10T17:57:28Z

frontend/public/components/monitoring/hooks/useIsVisible.ts

@@ -11,6 +11,7 @@ export const useIsVisible = (ref) => {
    }
  };

+  // eslint-disable-next-line react-hooks/exhaustive-deps


Why was this needed?

spadgett · 2021-11-10T17:58:59Z

We'll need to solve this error:

error @typescript-eslint/[email protected]: The engine "node" is incompatible with this module. Expected version "^12.22.0 || ^14.17.0 || >=16.0.0". Got "14.16.0"

glekner · 2021-11-10T23:34:26Z

@spadgett

now upgrading @typescript-eslint/eslint-plugin and @typescript-eslint/parser to v4 instead of v5. v5 is mainly for ESLint 8 which we don't use. our node version is now supported
camelcase is deprecated in favor of https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/naming-convention.md
Note, all useCallback eslint disabling is because the plugin prefer inline functions instead of external ones even though they are completely fine.
cx usages also needed eslint disabling for naming-convention because of the classnames

rebeccaalpert · 2021-11-23T18:46:45Z

/go/src/github.com/openshift/console/frontend/node_modules/ts-node/src/index.ts:750
    return new TSError(diagnosticText, diagnosticCodes);
           ^
TSError: ⨯ Unable to compile TypeScript:
scripts/generate-pkg-assets.ts(20,10): error TS2550: Property 'entries' does not exist on type 'ObjectConstructor'. Do you need to change your target library? Try changing the `lib` compiler option to 'es2017' or later.
    at createTSError (/go/src/github.com/openshift/console/frontend/node_modules/ts-node/src/index.ts:750:12)
    at reportTSError (/go/src/github.com/openshift/console/frontend/node_modules/ts-node/src/index.ts:754:19)
    at getOutput (/go/src/github.com/openshift/console/frontend/node_modules/ts-node/src/index.ts:941:36)
    at Object.compile (/go/src/github.com/openshift/console/frontend/node_modules/ts-node/src/index.ts:1243:30)
    at Module.m._compile (/go/src/github.com/openshift/console/frontend/node_modules/ts-node/src/index.ts:1370:30)
    at Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Object.require.extensions.<computed> [as .ts] (/go/src/github.com/openshift/console/frontend/node_modules/ts-node/src/index.ts:1374:12)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12) {
  diagnosticText: "scripts/generate-pkg-assets.ts(20,10): error TS2550: Property 'entries' does not exist on type 'ObjectConstructor'. Do you need to change your target library? Try changing the `lib` compiler option to 'es2017' or later.\n",
  diagnosticCodes: [ 2550 ]

rebeccaalpert · 2021-11-23T18:57:47Z

I spoke to Kim and she suggested creating a branch based on glekner:ts4/a new PR and then close this. I'll take a look at the GH comments before I do that to make sure they have been addressed first.

glekner · 2021-11-25T13:24:34Z

@rebeccaalpert I fixed the TS compilation error.

The only remaining issue AFAIK is jest. I assume the current version of ts-jest doesn't support TS4.
and upgrading it means upgrading the rest of the jest pkgs.

openshift-ci · 2021-11-25T15:41:46Z

@glekner: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/frontend	`778c1da`	link	true	`/test frontend`
ci/prow/e2e-gcp-console	`778c1da`	link	true	`/test e2e-gcp-console`
ci/prow/ceph-storage-plugin	`778c1da`	link	true	`/test ceph-storage-plugin`
ci/prow/kubevirt-plugin	`778c1da`	link	true	`/test kubevirt-plugin`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-ci · 2021-11-29T03:17:36Z

@glekner: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

rebeccaalpert · 2021-12-02T18:23:19Z

Closing this per Kim. I will work on fixing tests and resolving open comments in this PR in the new PR.

openshift-ci bot added component/ceph Related to ceph-storage-plugin component/core Related to console core functionality labels Nov 10, 2021

openshift-ci bot requested review from afreen23 and andybraren November 10, 2021 17:07

glekner force-pushed the ts4 branch 2 times, most recently from e415e7f to 3f993e3 Compare November 10, 2021 17:30

spadgett reviewed Nov 10, 2021

View reviewed changes

glekner force-pushed the ts4 branch from 3f993e3 to 4cd9ba8 Compare November 10, 2021 23:34

glekner force-pushed the ts4 branch from 4cd9ba8 to 14a8e63 Compare November 11, 2021 09:46

glekner mentioned this pull request Nov 11, 2021

Use clean PF components for building Overview cards #10410

Merged

spadgett changed the title ~~Upgrade to Typescript v4~~ CONSOLE-2501: Upgrade to Typescript v4 Nov 11, 2021

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 14, 2021

glekner force-pushed the ts4 branch from 14a8e63 to d039a62 Compare November 14, 2021 10:05

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 14, 2021

glekner force-pushed the ts4 branch from d039a62 to ab5d3ff Compare November 14, 2021 10:42

openshift-ci bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 21, 2021

glekner force-pushed the ts4 branch from a44a768 to 8ea466c Compare November 21, 2021 11:15

glekner force-pushed the ts4 branch from 8ea466c to 7229ca1 Compare November 25, 2021 12:01

bump typescript to v4.2.4

778c1da

glekner force-pushed the ts4 branch from 7229ca1 to 778c1da Compare November 25, 2021 12:23

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 29, 2021

rebeccaalpert mentioned this pull request Dec 2, 2021

[WIP] CONSOLE-2501: Bump Typescript to v4.2.4 #10604

Closed

rebeccaalpert closed this Dec 2, 2021

vojtechszocs mentioned this pull request May 16, 2023

CONSOLE-2501: Upgrade TypeScript version to 4.5 #12821

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CONSOLE-2501: Upgrade to Typescript v4 #10432

CONSOLE-2501: Upgrade to Typescript v4 #10432

glekner commented Nov 10, 2021 •

edited

Loading

glekner commented Nov 10, 2021

openshift-ci bot commented Nov 10, 2021

spadgett Nov 10, 2021

spadgett Nov 10, 2021

glekner Nov 10, 2021

spadgett Nov 11, 2021

spadgett Nov 10, 2021

spadgett Nov 10, 2021

spadgett Nov 10, 2021

spadgett Nov 10, 2021

spadgett commented Nov 10, 2021

glekner commented Nov 10, 2021

rebeccaalpert commented Nov 23, 2021

rebeccaalpert commented Nov 23, 2021

glekner commented Nov 25, 2021

openshift-ci bot commented Nov 25, 2021

openshift-ci bot commented Nov 29, 2021

rebeccaalpert commented Dec 2, 2021

		@@ -1,3 +1,4 @@
		/* eslint-disable react-hooks/exhaustive-deps */

CONSOLE-2501: Upgrade to Typescript v4 #10432

CONSOLE-2501: Upgrade to Typescript v4 #10432

Conversation

glekner commented Nov 10, 2021 • edited Loading

glekner commented Nov 10, 2021

openshift-ci bot commented Nov 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spadgett commented Nov 10, 2021

glekner commented Nov 10, 2021

rebeccaalpert commented Nov 23, 2021

rebeccaalpert commented Nov 23, 2021

glekner commented Nov 25, 2021

openshift-ci bot commented Nov 25, 2021

openshift-ci bot commented Nov 29, 2021

rebeccaalpert commented Dec 2, 2021

glekner commented Nov 10, 2021 •

edited

Loading