CONSOLE-4675: Enable strictNullChecks: true for olm and olm-v1

[krishagarwal278]

Key Guidelines

When addressing TypeScript errors related to strictNullChecks, keep the following guiding principles in mind:

Type Matching: Ensure that parent and child component props and their corresponding types are always consistent and correctly matched.

Avoid Ambiguous Types: Do not use overly broad or ambiguous types such as any, and do not use type assertions using the keyword as. NEVER assert objects, variables, or parameters using as any. Using these types defeats the type-checking purpose of TypeScript and can lead to runtime errors that are difficult to debug.

Enforce Strict Typing: Always use the correct and most specific type for a variable or prop. For example, if a prop is a string, its type should be string, not undefined.

Favor Optional Chaining: Prefer using optional chaining (?.) for accessing nested properties. This practice improves code readability and prevents errors when a property might be null or undefined.

Use precise Kubernetes types: When dealing with Kubernetes resources, use the most precise types available to ensure accurate configuration and interaction with the API. We're frequently using generic types like K8sResourceCommon, which define metadata.name and metadata.namespace as optional. However, in many of our use cases, we know these properties exist. name is almost always required, and namespace is mandatory for namespaced resources. Instead of littering the code with optional chaining (?.) to access them, let's use or create more specific types that accurately reflect the resource's shape. This strengthens our type safety and removes unnecessary null checks.

Fix the Root Cause, Not Just the Symptom: Avoid using union types (| null), type casting (as), and any to resolve compiler errors. While effective, these can sometimes act as a band-aid, masking deeper issues in our type definitions. Let's treat a type error as a signal to investigate the underlying data model. Instead of forcing a type to fit, let's ask why the type is incorrect and aim to fix it at the source.

Initialize with Fallbacks, Don't Handle Nulls Repeatedly: Let's define sane default values at initialization instead of applying nullish coalescing (??) or logical OR (||) fallbacks every time a variable is used. This makes the variable's state predictable and reduces redundant code. This is especially helpful for deeply nested properties—extract them into a clearly named variable with a default.

Favor Optional Chaining for Readability: When destructuring potentially nullish objects, providing a fallback object (e.g., (foo ?? {})) can obscure the code's intent. A more readable and explicit approach is to use the JavaScript optional chaining operator (?.) for each property. This clearly communicates that the source object might be null and provides safe defaults for each value independently.

Use OpenShift console-specific idioms and types: In new code, OpenShift console uses specific types such as React.FCC instead of React.FC to type components, to fix certain issues with legacy dependencies. Prefer using React.FCC when fixing types issues related to components returning null.

[openshift-ci-robot]

@krishagarwal278: This pull request references CONSOLE-4675 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "4.20.0" version, but no target version was set.

In response to this:

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 openshift-eng/jira-lifecycle-plugin repository.

[krishagarwal278]

/label tide/merge-method-squash

[krishagarwal278]

/retest

[krishagarwal278]

/label docs-approved
/label px-approved

[jhadvig]

/retest

[jhadvig]

Two nist, otherwise should be ready to go 👍

[jhadvig]

nit: I think we could just defer to the else statement here and directly set

      obj.spec = {
        resources: {
          limits: { memory: '50Mi', cpu: '500m', 'ephemeral-storage': '50Mi' },
          requests: { memory: '50Mi', cpu: '500m', 'ephemeral-storage': '50Mi' },
        },
      };

[krishagarwal278]

addressed in latest commit

[jhadvig]

nit: same as above

[yanpzhan]

Regression test passed on cluster launched against the pr.
/label qe-approved

[openshift-ci-robot]

@krishagarwal278: This pull request references CONSOLE-4675 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "4.20.0" version, but no target version was set.

In response to this:

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 openshift-eng/jira-lifecycle-plugin repository.

[jhadvig]

panic: Error waiting program :./testdata/cacertCreate.sh::exit status 1
goroutine 1 [running]:
github.com/openshift/console/pkg/helm/actions.TestMain(0xc000708640)
	/home/prow/go/src/github.com/openshift/console/pkg/helm/actions/setup_test.go:24 +0x10e
main.main()
	_testmain.go:109 +0xa8
FAIL	github.com/openshift/console/pkg/helm/actions	16.506s

/retest

[logonoff]

i think you'll have to do some adjustments after the last merge commit..

[logonoff]

Suggested change

	const [done, setDone] = React.useState(false);
	const [error, setError] = React.useState<Error | null>(null);
	const refresh = React.useRef(
	const [done, setDone] = useState(false);
	const [error, setError] = useState<Error | null>(null);
	const refresh = useRef(

[logonoff]

you get the idea..

Suggested change

	const { done: initDone, error: initError } = React.useContext(ExtensionCatalogDatabaseContext);
	const [categories, setCategories] = React.useState<IDBValidKey[]>([]);
	const [loading, setLoading] = React.useState(!initDone);
	const [error, setError] = React.useState<Error | null>(initError ?? null);
	const { done: initDone, error: initError } = useContext(ExtensionCatalogDatabaseContext);
	const [categories, setCategories] = useState<IDBValidKey[]>([]);
	const [loading, setLoading] = useState(!initDone);
	const [error, setError] = useState<Error | null>(initError ?? null);

[Leo6Leo]

@krishagarwal278 There are few build errors need to get fixed first, feel free to ping me again whenever those errors have been fixed, thanks!

The commands to show all the build errors:

cd console/frontend
yarn lint

The result running from my end:

~ path/console/frontend/packages/operator-lifecycle-manager/src/components/subscription.tsx
  222:7  error  'menuActions' is assigned a value but never used  @typescript-eslint/no-unused-vars

~ path/console/frontend/packages/operator-lifecycle-manager-v1/src/contexts/useExtensionCatalogDatabaseContextValues.ts
  1:10  error  'useState' is defined but never used  @typescript-eslint/no-unused-vars
  1:20  error  'useRef' is defined but never used    @typescript-eslint/no-unused-vars

~ path/console/frontend/packages/operator-lifecycle-manager-v1/src/hooks/useExtensionCatalogCategories.ts
  1:10  error  'useContext' is defined but never used  @typescript-eslint/no-unused-vars
  1:22  error  'useState' is defined but never used    @typescript-eslint/no-unused-vars
  1:56  error  'useMemo' is defined but never used     @typescript-eslint/no-unused-vars

~ path/console/frontend/packages/operator-lifecycle-manager-v1/src/hooks/useExtensionCatalogItems.ts
  1:10  error  'useContext' is defined but never used  @typescript-eslint/no-unused-vars
  1:22  error  'useState' is defined but never used    @typescript-eslint/no-unused-vars

~ path/console/frontend/public/components/cluster-settings/global-config.tsx
  187:6  warning  React Hook React.useEffect has missing dependencies: 'editYAMLMenuItem', 'oauthMenuItems', and 'viewAPIExplorerMenuItem'. Either include them or remove the dependency array  react-hooks/exhaustive-deps

~ path/console/frontend/public/components/configmaps/ConfigMapFormEditor.tsx
  80:6  warning  React Hook React.useCallback has missing dependencies: 'editorType', 'formData.namespace', and 'formReloadCount'. Either include them or remove the dependency array  react-hooks/exhaustive-deps

~ path/console/frontend/public/components/droppable-edit-yaml.tsx
  128:5  warning  React Hook useCallback has an unnecessary dependency: 'maxFileUploadSize'. Either exclude it or remove the dependency array. Outer scope values like 'maxFileUploadSize' aren't valid dependencies because mutating them doesn't re-render the component  react-hooks/exhaustive-deps

~ path/console/frontend/public/components/modals/edit-yaml-settings-modal.tsx
  26:31  warning  tsdoc-undefined-tag: The TSDoc tag "@fontawesome" is not defined in this configuration  tsdoc/syntax
  39:31  warning  tsdoc-undefined-tag: The TSDoc tag "@fontawesome" is not defined in this configuration  tsdoc/syntax

~ path/console/frontend/public/components/utils/console-select.tsx
  187:5  warning  React Hook React.useCallback has an unnecessary dependency: 'props.items'. Either exclude it or remove the dependency array            react-hooks/exhaustive-deps
  218:6  warning  React Hook React.useEffect has a missing dependency: 'props.items'. Either include it or remove the dependency array                   react-hooks/exhaustive-deps
  239:6  warning  React Hook React.useMemo has an unnecessary dependency: 'onClick'. Either exclude it or remove the dependency array                    react-hooks/exhaustive-deps
  285:6  warning  React Hook React.useMemo has unnecessary dependencies: 'onClick' and 'storageKey'. Either exclude them or remove the dependency array  react-hooks/exhaustive-deps

✖ 17 problems (8 errors, 9 warnings)

[Leo6Leo]

There are few build errors that will need to be addressed before proceeding with the review. This is one of the example.

TS2304: Cannot find name 'KebabAction'.

[krishagarwal278]

/retest

[TheRealJon]

Hi Krish, great work on tackling the strict null checks! This is a huge step forward for our codebase's stability. 👍

As we're all working on this, I've gathered a few guiding principles to help us stay consistent and ensure our solutions are robust and readable.

1. Fix the Root Cause, Not Just the Symptom

I've noticed a pattern of using union types (| null), type casting (as), and any to resolve compiler errors. While effective, these can sometimes act as a band-aid, masking deeper issues in our type definitions.

Let's treat a type error as a signal to investigate the underlying data model. Instead of forcing a type to fit, let's ask why the type is incorrect and aim to fix it at the source.

2. Initialize with Fallbacks, Don't Handle Nulls Repeatedly

Let's define sane default values at initialization instead of applying nullish coalescing (??) or logical OR (||) fallbacks every time a variable is used. This makes the variable's state predictable and reduces redundant code.

This is especially helpful for deeply nested properties—extract them into a clearly named variable with a default.

// ✅ Prefer: Initialize with a safe fallback once.
const fuga = hoge?.fuga ?? '';
console.log('fuga is:', fuga);
return fuga;

// ❌ Avoid: Repetitive null checks.
const fuga = hoge?.fuga;
console.log('fuga is:', fuga ?? '');
return fuga ?? '';

3. Favor Optional Chaining for Readability

When destructuring potentially nullish objects, providing a fallback object (e.g., hoge ?? {}) can obscure the code's intent. A more readable and explicit approach is to use optional chaining (?.) for each property. This clearly communicates that the source object might be null and provides safe defaults for each value independently.

// ✅ Prefer: Clear, explicit, and safe.
const fuga = hoge?.fuga ?? '';
const piyo = hoge?.piyo ?? '';
return [fuga, piyo];

// ❌ Avoid: Can hide the nullable nature of the source object.
const { fuga, piyo } = hoge ?? { fuga: '', piyo: '' };
return [fuga, piyo];

4. Use Precise Types for Kubernetes Resources

We're frequently using generic types like K8sResourceCommon, which define metadata.name and metadata.namespace as optional. However, in many of our use cases, we know these properties exist. name is almost always required, and namespace is mandatory for namespaced resources.

Instead of littering the code with optional chaining (?.) to access them, let's use or create more specific types that accurately reflect the resource's shape. This strengthens our type safety and removes unnecessary null checks.

5. On AI-Generated Code: Trust but Verify 🤖

AI assistants like Cursor are perfect for tasks like this. However, it can sometimes generate suboptimal code for our specific context.

Please treat AI-generated code with the same critical eye as you would any other code. Let's ensure we fully understand and take ownership of every line we commit, regardless of its origin.

These are just suggestions to guide our efforts. Let me know what you think! Overall, this is fantastic progress. ✨

---

Full disclosure - I used Gemini to refine this PR review comment.

Also, as an idea, maybe you could compile comments from your snc PR reviews into context for Cursor which will probably greatly improve results.

[TheRealJon]

Suggested change

	const [error, setError] = React.useState<Error | null>(initError ?? null);
	const [error, setError] = React.useState(initError.toString() || '');

[TheRealJon]

Suggested change

	export const useExtensionCatalogCategories = (): [CatalogCategory[], boolean, Error] => {
	export const useExtensionCatalogCategories = (): [CatalogCategory[], boolean, string] => {

[TheRealJon]

Presumably, when error is null, this means no errors have occurred. With this change, we would always return an error.

Suggested change

	return [catalogCategories, loading, error ?? new Error('Unknown error')];
	return [catalogCategories, loading, error];

[TheRealJon]

Import the To type from react router instead of casting this to any:

Suggested change

	const to =
	const to: To =

[TheRealJon]

Suggested change

	<Link className="csv-operator-status__title" to={to as any}>
	<Link className="csv-operator-status__title" to={to}>

[TheRealJon]

Suggested change

	const { namespace } = resource?.metadata || {};
	const namespace = resource?.metadata?.namespace ?? '';

[TheRealJon]

Suggested change

	const { namespace } = resource?.metadata || {};
	const namespace = resource?.metadata?.namespace || '';

[TheRealJon]

Suggested change

	const { name } = csv.metadata || {};
	const name = csv.metadata?.name || '';

[TheRealJon]

We should always know that hard-coded static test data exists. We probably need to break down this test data into smaller chunks so that we aren't trying to use deeply nested, optional properties like this. Instead, we factor out the parts we want into a building blocks, and reference those parts where we need them.

[TheRealJon]

I believe class would be a string type, so we should use empty string instead of null or undefined

Suggested change

	class: undefined,
	class: '',

[openshift-ci-robot]

@krishagarwal278: This pull request references CONSOLE-4675 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "4.21.0" version, but no target version was set.

In response to this:

Key Guidelines

When addressing TypeScript errors related to strictNullChecks, keep the following guiding principles in mind:

Type Matching: Ensure that parent and child component props and their corresponding types are always consistent and correctly matched.

Avoid Ambiguous Types: Do not use overly broad or ambiguous types such as any, and do not use type assertions using the keyword as. NEVER assert objects, variables, or parameters using as any. Using these types defeats the type-checking purpose of TypeScript and can lead to runtime errors that are difficult to debug.

Enforce Strict Typing: Always use the correct and most specific type for a variable or prop. For example, if a prop is a string, its type should be string, not undefined.

Favor Optional Chaining: Prefer using optional chaining (?.) for accessing nested properties. This practice improves code readability and prevents errors when a property might be null or undefined.

Use precise Kubernetes types: When dealing with Kubernetes resources, use the most precise types available to ensure accurate configuration and interaction with the API. We're frequently using generic types like K8sResourceCommon, which define metadata.name and metadata.namespace as optional. However, in many of our use cases, we know these properties exist. name is almost always required, and namespace is mandatory for namespaced resources. Instead of littering the code with optional chaining (?.) to access them, let's use or create more specific types that accurately reflect the resource's shape. This strengthens our type safety and removes unnecessary null checks.

Fix the Root Cause, Not Just the Symptom: Avoid using union types (| null), type casting (as), and any to resolve compiler errors. While effective, these can sometimes act as a band-aid, masking deeper issues in our type definitions. Let's treat a type error as a signal to investigate the underlying data model. Instead of forcing a type to fit, let's ask why the type is incorrect and aim to fix it at the source.

Initialize with Fallbacks, Don't Handle Nulls Repeatedly: Let's define sane default values at initialization instead of applying nullish coalescing (??) or logical OR (||) fallbacks every time a variable is used. This makes the variable's state predictable and reduces redundant code. This is especially helpful for deeply nested properties—extract them into a clearly named variable with a default.

Favor Optional Chaining for Readability: When destructuring potentially nullish objects, providing a fallback object (e.g., (foo ?? {})) can obscure the code's intent. A more readable and explicit approach is to use the JavaScript optional chaining operator (?.) for each property. This clearly communicates that the source object might be null and provides safe defaults for each value independently.

Use OpenShift console-specific idioms and types: In new code, OpenShift console uses specific types such as React.FCC instead of React.FC to type components, to fix certain issues with legacy dependencies. Prefer using React.FCC when fixing types issues related to components returning null.

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 openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: krishagarwal278
Once this PR has been reviewed and has the lgtm label, please assign christoph-jerolimov for approval. For more information see the Code Review Process.

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

[TheRealJon]

I stopped at [frontend/packages/operator-lifecycle-manager/src/components/operand/operand-form.tsx. This still isn't quite ready. There are still a lot of instances of any and type casting that need to be addressed.

[TheRealJon]

This and many of the olmv1 files will be removed as part of my current story, you can just add a ts-ignore comment to this file and a comment saying that it will be removed as part of https://issues.redhat.com//browse/CONSOLE-4668

[TheRealJon]

ts-ingore, will be removed as part of https://issues.redhat.com//browse/CONSOLE-4668

[TheRealJon]

ts-ingore, will be removed as part of https://issues.redhat.com//browse/CONSOLE-4668

[TheRealJon]

ts-ingore, will be removed as part of https://issues.redhat.com//browse/CONSOLE-4668

[TheRealJon]

ts-ingore, will be removed as part of https://issues.redhat.com//browse/CONSOLE-4668

[TheRealJon]

don't use any

[TheRealJon]

don't use any

[TheRealJon]

don't use any

[TheRealJon]

don't use any

[TheRealJon]

don't use any

[openshift-ci]

@krishagarwal278: 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	8b8d7e9	link	true	/test frontend
ci/prow/e2e-gcp-console	8b8d7e9	link	true	/test e2e-gcp-console

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-sigs/prow repository. I understand the commands that are listed here.