Start using @typescript-eslint/no-unnecessary-condition eslint rule

[grigasp]

Added @typescript-eslint/no-unnecessary-condition eslint rule, which found a number of unnecessary checks, especially optional chaining and nullish coalescing.

3 cases when I had to unnecessarily make changes to our code to overcome it:

1. Checking if object is of type, based on derived type attribute (usually in type guards):

   // this used to work, but now eslint complains:
   function isFoo(obj: BaseType): obj is DerivedType {
     return (obj as DerivedType).derivedAttribute !== undefined;
   }
   // do this instead:
   function isFoo(obj: BaseType): obj is DerivedType {
     return "derivedAttribute" in obj;
   }

2. Value changes in functions:

    useEffect(() => {
      let disposed = false;
      asyncFunction().then((result) => {
        // eslint complains that `disposed` is always `false` here, define it as `let disposed = false as boolean` instead,
        // this is one of the known issues with this rule
        if (!disposed) {
          setState(result);
        }
      });
      return () => {
         disposed = true;
      };
    }, [dependencies]);

3. Object property access:

   const value: {[key: string]: number} = {};
   // eslint complains that value is always truthy here, use `Map` instead
   if (!value) {
     // ...
   }

[changeset-bot]

🦋 Changeset detected

Latest commit: e93fd15

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages

Name	Type
@itwin/presentation-shared	Major
@itwin/presentation-hierarchies-react	Patch
@itwin/presentation-core-interop	Patch
@itwin/presentation-components	Patch
@itwin/presentation-testing	Patch
@itwin/presentation-hierarchies	Patch
@itwin/unified-selection	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[saskliutas]

// this used to work, but now eslint complains:
function isFoo(obj: BaseType): obj is DerivedType {
  return (obj as DerivedType).derivedAttribute !== undefined;
}
// do this instead:
function isFoo(obj: BaseType): obj is DerivedType {
  return "derivedAttribute" in obj;
}

Looks like these are not equivalent, Shouldn't second be "derivedAttribute" in obj && obj.derivedAttribute !== undefined to fully match?

[grigasp]

// this used to work, but now eslint complains:
function isFoo(obj: BaseType): obj is DerivedType {
  return (obj as DerivedType).derivedAttribute !== undefined;
}
// do this instead:
function isFoo(obj: BaseType): obj is DerivedType {
  return "derivedAttribute" in obj;
}

Looks like these are not equivalent, Shouldn't second be "derivedAttribute" in obj && obj.derivedAttribute !== undefined to fully match?

I agree they're not fully equivalent, however, for these type guard type of use cases, I think "derivedAttribute" in obj check is sufficient:

• if derivedAttribute is non-optional, then checking for its existence is enough,
• if it's optional, then I'd argue the attribute is not suitable for checking the type at all.

Do you see any specific case where not having && obj.derivedAttribute !== undefined would produce invalid result?

[saskliutas]

// this used to work, but now eslint complains:
function isFoo(obj: BaseType): obj is DerivedType {
  return (obj as DerivedType).derivedAttribute !== undefined;
}
// do this instead:
function isFoo(obj: BaseType): obj is DerivedType {
  return "derivedAttribute" in obj;
}

Looks like these are not equivalent, Shouldn't second be "derivedAttribute" in obj && obj.derivedAttribute !== undefined to fully match?

I agree they're not fully equivalent, however, for these type guard type of use cases, I think "derivedAttribute" in obj check is sufficient:

• if derivedAttribute is non-optional, then checking for its existence is enough,
• if it's optional, then I'd argue the attribute is not suitable for checking the type at all.

Do you see any specific case where not having && obj.derivedAttribute !== undefined would produce invalid result?

No, just wanted to make sure that we won't miss this. And I agree that we should not type check based on optional properties.

[Copilot]

Pull request overview

This PR enables the @typescript-eslint/no-unnecessary-condition ESLint rule to eliminate unnecessary null/undefined checks and optional chaining operations. The changes primarily remove redundant type guards, replace object property checks with in operator, convert object-based maps to Map instances, and add explicit type annotations where the rule cannot infer mutability.

Changes:

• Removed unnecessary optional chaining (?.) and nullish coalescing (??) operators where types guarantee non-null values
• Replaced object property type guards (obj.prop !== undefined) with the in operator for better type narrowing
• Converted plain object maps to Map instances to avoid always-truthy object references
• Added explicit boolean type annotations for variables modified in closures to work around rule limitations

Reviewed changes

Copilot reviewed 60 out of 60 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
eslint.base.config.js	Added the @typescript-eslint/no-unnecessary-condition rule configuration
packages/shared/src/shared/Metadata.ts	Made multiplicity required in RelationshipConstraint interface
packages/shared/src/shared/Values.ts	Changed Point2d/Point3d type guards to use in operator
packages/shared/src/test/MetadataProviderStub.ts	Converted schema stubs storage from object to Map
packages/testing/src/test/HierarchyBuilder.test.ts	Removed redundant optional chaining on opts parameter
packages/hierarchies-react/src/presentation-hierarchies-react/internal/UseUnifiedSelection.ts	Converted imodel selectables storage from object to Map
packages/hierarchies-react/src/presentation-hierarchies-react/stratakit/TreeRenderer.tsx	Removed unnecessary optional chaining and unused helper function
packages/hierarchies-react/src/presentation-hierarchies-react/UseTree.ts	Added explicit boolean type annotation for closure variable
packages/components/src/presentation-components/propertygrid/DataProvider.ts	Removed unnecessary null checks and added version compatibility comments
apps/test-app/frontend/src/index.tsx	Simplified promise array initialization by removing undefined check

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

@grigasp I've opened a new pull request, #1201, to work on those changes. Once the pull request is ready, I'll request review from you.