Skip to content

[Refactor]: Replace cast in causes/index.ts with a structural guard #74

Description

@martyy-code

Current State

In packages/errors/src/causes/index.ts:30, the function-scope variable instance is used as the local name for what is actually a narrowed ErrorInstance:

const causes = (error: unknown): Error[] => {
  if (error == null) {
    return [];
  }

  // Get the causes array from the error
  const instance = error as ErrorInstance;

Rule 0005 (Named Algorithms and Independent Data Structures) treats project-internal diminutives — names that train the reader to translate every line — as a tax. instance is a JavaScript keyword-adjacent word that does not name a concept; in this function it is the receiver of an unsafe cast. The function is 13 lines long; the name carries no domain meaning.

Located in:

  • packages/errors/src/causes/index.ts:30, 36, 38

Problems with current implementation:

  • instance is not a domain concept; it is a JavaScript-shape term. The function deals with errors, not instances.
  • The reader has to remember that instance means "the error we just cast" rather than "an instance of something" — the cast is the only thing giving the name meaning.
  • The neighbouring parameter is named error; instance next to it is redundant.

Proposed State

After refactoring, the variable is renamed to errorInstance (full form, no truncation) and the function uses an early-return that makes the narrowing explicit:

const causes = (error: unknown): Error[] => {
  if (error == null) {
    return [];
  }
  if (!('causes' in error) || !Array.isArray(error.causes)) {
    return [];
  }
  return error.causes;
}

The shape of the cast is preserved (as ErrorInstance was unsound — the new code replaces it with a structural guard, which is the rule 0004 / 0008 cooperative fix).

Expected improvements:

  • Rule 0005 compliance: no project-internal diminutives.
  • Rule 0004 compliance: the runtime guard names its scenario ("causes is an array") instead of relying on a cast.
  • Rule 0008 compliance: the cast at the call site is removed.
  • The function reads top-down (rule 0007): each branch is a name, no cast survives the null guard.

Motivation

This refactoring is needed because:

  • The variable is a diminutive that a reader has to mentally expand on every read.
  • The function is exported as part of the public API (causes); the name carries weight for consumers who skim the source.
  • Combining the rename with the structural-guard refactor kills three birds (0005, 0004, 0008) in one pass.

Triggers for this work:

  • Technical debt accumulation
  • Maintainability concerns

Risks

Potential risks:

  • Risk 1: A consumer relies on causes() returning [] for inputs that are not null but also don't have a causes array. — Mitigation: the current code does instance as ErrorInstance then checks Array.isArray(instance.causes). If the cast succeeds (the input has a causes property of any shape), the function returns that shape. The new code preserves the same behaviour through a structural check.
  • Risk 2: The error.causes property does not exist on native Error instances. — Mitigation: the early return if (!('causes' in error) || !Array.isArray(error.causes)) return []; covers this; native errors return [] as before.

Migration Plan

Migration approach:

  1. Replace the cast + Array.isArray check with a single structural guard.
  2. Rename instance to errorInstance (or remove the local entirely by inlining the structural check, which the early-return shape makes natural).
  3. Update tests if they assert on the cast's runtime behaviour.

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

Backward Compatibility

  • This refactoring maintains full backward compatibility

Scope

Files/Folders affected:

  • packages/errors/src/causes/index.ts (the function body)
  • packages/errors/tests/ (if any test exercises the cast path)

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

Verification steps:

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

Related Issues / Pull Requests

  • Related audit: P0 feat: implement .from() method for exception chaining #4 in the internal audit of packages/errors/src/ against rules 0001-0016, August 2026.
  • Related rule: docs/engineering/architecture/rules/0005-named-algorithms-and-independent-data-structures.md.
  • Related rule: docs/engineering/architecture/rules/0004-no-speculative-defences.md.
  • Related rule: docs/engineering/architecture/rules/0008-no-chained-type-assertions.md.

Relevant Documentation

  • Architecture doc: docs/engineering/architecture/rules/0005-named-algorithms-and-independent-data-structures.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)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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