Skip to content

[Refactor]: Remove redundant typeof/null guard after narrowing in is/index.ts #73

Description

@martyy-code

Current State

In packages/errors/src/is/index.ts:60-78, the is() function performs a null check on error at the top of the function, then re-checks typeof error === 'object' && error !== null ten lines later before accessing error[FACTORY_SYMBOL]. The compiler has already narrowed error to object; the second check is redundant and reads as the author having lost confidence in their own code.

// Line 60-63
if (error == null) {
  return false;
}

// Line 77-78
if (typeof error === 'object' && error !== null) {
  const marker = error as Record<typeof FACTORY_SYMBOL, unknown>;

Rule 0004 names this exact pattern in its "What this looks like in violation" section:

"A function declared with err: unknown that, three statements later, re-checks typeof err === 'object' && err !== null even though a prior branch already returned on err == null. The author was not sure the narrowing survived the intervening statements."

Located in:

  • packages/errors/src/is/index.ts:77-78

Problems with current implementation:

  • The compiler has the narrowing. The re-check is a tax on every reader who has to confirm the second check is dead.
  • It mirrors the canonical violation example in rule 0004.
  • The pattern signals "the author didn't trust their own type narrowing", which propagates distrust to the next contributor.

Proposed State

After refactoring, the re-check is removed. The function accesses error[FACTORY_SYMBOL] directly after the top-level null guard:

if (error == null) {
  return false;
}

// Native errors
if (typeof ErrorType === 'function' && 'prototype' in ErrorType) {
  return error instanceof ErrorType;
}

// Factory instances
if (typeof error === 'object') {
  const marker = error as Record<typeof FACTORY_SYMBOL, unknown>;
  // ...
}

Expected improvements:

  • The narrowing is trusted (rule 0004's trust-the-type principle).
  • One fewer branch in the hot path.
  • The reader can follow the narrowing without re-auditing the second check.

Motivation

This refactoring is needed because:

  • It is the canonical violation example in rule 0004.
  • The project slogan ("trust the type") is operationalised by trusting the narrowing; this code does the opposite.
  • The redundant branch is noise that hides the actual logic of the function.

Triggers for this work:

  • Working on feature X and encountered this (audit of packages/errors/src/ against the 16 architecture rules, August 2026)
  • Technical debt accumulation
  • Performance issues
  • Maintainability concerns

Risks

Potential risks:

  • Risk 1: TypeScript's narrowing across the instanceof block may be lost, causing a new type error on error[FACTORY_SYMBOL]. — Mitigation: the compiler tracks narrowing across early-returns and type guards. If a regression appears, the fix is a single typeof error === 'object' cast at the access point, not the re-check.
  • Risk 2: Consumer code paths where error is null reach the second block. — Mitigation: the top-level if (error == null) return false; covers this. The re-check does not add safety.

Migration Plan

Migration approach:

  1. Remove the typeof error === 'object' && error !== null wrapper.
  2. Run pnpm --filter @deessejs/errors type-check to confirm the narrowing is preserved.
  3. Run the test suite to confirm runtime behaviour is unchanged.

Rollback plan: revert the PR. The change is local.

Backward Compatibility

  • This refactoring maintains full backward compatibility

Scope

Files/Folders affected:

  • packages/errors/src/is/index.ts (lines 77-78)

Out of scope:

Component(s) Affected

  • Multiple Components

Note: the component_affected dropdown is calibrated for a web template project. The actual affected component is packages/errors.

Priority

  • p0: Critical - Blocking major work or causing bugs
  • p1: High - Important, should do soon
  • p2: Medium - Normal priority
  • p3: Low - Nice to have

Estimated Effort

  • effort: xs - Few minutes

Test Coverage Requirements

  • Existing tests cover this code area (will update)

Testing Approach

Testing strategy:

  • Unit tests: existing is() test cases (factory instances, native errors, null/undefined) should all pass without modification.

Verification steps:

  1. pnpm --filter @deessejs/errors test:run
  2. pnpm --filter @deessejs/errors type-check

Related Issues / Pull Requests

Relevant Documentation

  • Architecture doc: docs/engineering/architecture/rules/0004-no-speculative-defences.md

Pre-Submission Checklist

  • I have searched existing issues for related refactoring requests
  • Risks and migration plan are documented
  • Test coverage approach is defined
  • I understand this issue will be labeled according to the project taxonomy
  • This is NOT a security vulnerability (see security note above)

Metadata

Metadata

Assignees

No one assigned

    Labels

    p1: highRequired for next releasetype: refactorRefactoring / code restructuring

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions