Repository navigation
Description
Context
The lead's review of PR #87 (branded ErrorInstance) noted that the implementation is structurally OOP (carries methods, owns mutable state, has identity) but written in a functional style that requires casts and symbols to simulate class semantics. Three options were proposed:
- Phantom brand: 0 octet runtime, type-only. Loses the runtime guard capability that
FACTORY_SYMBOLalready provides. - Branding runtime via object literal: builds the object in one pass with the brand as a literal key. Cleaner than the current
new Error()+ cast + helper pattern. Pure refactor; preserves the FP API. - Class-based internals, FP externals: a private class implements the methods and the brand; the public API is the factory function that returns a typed object (not the class).
Option 2 lands cleanly inside PR #87. Option 3 is a structural shift that deserves its own decision.
Decision to evaluate
The lead suggested moving the implementation to OOP while keeping the API purely functional. Concretely:
- A private class (
ErrorInstanceImpl<TFields>) owns the methods (from,addNote), the mutable state (notes,causes,context), and the brand assignment. - The factory
error()instantiates the class internally, binds the public-facing methods, and returns a typed object whose type is exposed but whose class is not. - The public API (
error,is,causes,raise) is unchanged: factory functions return typed values, nonewis exposed.
This would resolve several smells from the current shape:
- The brand-instance helper (
brandInstance(instance)) becomes the class constructor's job. The cast on the brand property disappears. - The
addNote/frommethods, currently attached viainstance.addNote = ...with no type narrowing, are real class methods with properthistypes. - The brand is set in the constructor (the only assignment site), making it impossible to construct an
ErrorInstancethat lacks the brand. - Rule 0014's "no
export class" remains satisfied: the class is internal to the module; only the factory function is exported.
Trade-offs
Easier:
- Removes the
ascast at the construction site. - Methods get
thistypes instead of capturinginstanceviaconst. - The brand becomes constructor-enforced, not a property assignment after the fact.
- Aligns with how mature error libraries (VError, http-errors, Node's own ERR_* patterns) implement this internally.
Harder:
- The bundle shape changes: the class survives in the compiled output, but only the factory call sites see it. Tree-shaking removes the class symbol after the factory call returns (the returned value is an object, not the class). Bundle size impact: minimal.
- Rule 0014 is satisfied by keeping the class non-exported. This requires discipline — any temptation to
export class ErrorInstanceImplfor "convenience" would violate the rule. The naming convention (Impl suffix, never exported) is the guard. - The brand property becomes constructor-enforced. If a future contributor wants to bypass the constructor (e.g. for testing), they have to either reflect the brand manually or use a testing helper. The current
brandInstancehelper from PR feat(errors): brand ErrorInstance so the type can be trusted at boundaries #87 is the testing escape hatch and should be preserved as a private export.
Stack interaction
PR #87 is the bottom of a 3-PR sequence described in its description:
- feat(errors): brand ErrorInstance so the type can be trusted at boundaries #87 (in review) — brand
ErrorInstance<T>. Pure addition. - (next) — implement
is()predicate (issue [BUG] is() should return TypeScript type predicate for narrowing #35). Brand becomes reachable to consumers. - (later) — shrink public function parameters, retire dead guards.
The class-based refactor would sit at position 0 of the stack, before #87. It would obsolete the brandInstance helper (the constructor sets the brand) and the as cast (the class is the type). The brand-as-symbol concept survives — the class declares [ErrorInstanceBrand]: 'ErrorInstance' on its instances — but the construction shape becomes natural.
The cleanest sequence is therefore:
- Refactor
error()to a class internally, ship as a separate PR (this issue). - Re-implement PR feat(errors): brand ErrorInstance so the type can be trusted at boundaries #87 on the new construction site. The brand declaration survives; the
brandInstancehelper dies. - Land [BUG] is() should return TypeScript type predicate for narrowing #35 predicate.
- Land the cleanup PRs ([Refactor]: Remove silent try/catch around instanceof in is/index.ts #72, [Refactor]: Remove redundant typeof/null guard after narrowing in is/index.ts #73, [Refactor]: Replace cast in causes/index.ts with a structural guard #74 shrink).
Scope
packages/errors/src/error/error.ts— replace the inner functional implementation with a private class. The publicerror()factory signature is unchanged.packages/errors/src/error/types.ts—ErrorInstance<T>becomesInstanceType<typeof ErrorInstanceImpl>(the public type is derived from the class).packages/errors/src/is/index.ts—is()can drop its structural checks if the brand is a class-checkable nominal marker. (This is the cleanup PR [Refactor]: Remove redundant typeof/null guard after narrowing in is/index.ts #73 / [Refactor]: Remove silent try/catch around instanceof in is/index.ts #72.)- Tests — every test passes; the runtime behavior is identical.
Revisit conditions
This issue should be revisited when:
- A class is needed in the public API for any reason (rule 0014 carve-out expansion). Then this refactor's discipline needs revisiting.
- The brand-property pattern proves insufficient for some new feature (e.g. prototype-chain introspection,
Symbol.hasInstancecustomisation). - A bundle-size audit flags the class as a regression.
Effort and priority
- Effort:
m - 1-2 days. The class is straightforward; the tests pass unchanged. - Priority:
p2: medium. The class refactor is an improvement but not blocking. PR feat(errors): brand ErrorInstance so the type can be trusted at boundaries #87 lands the brand without it; the class work can follow in a focused PR. - Milestone:
arch-rules-auditif bundled with the brand work; otherwise a new milestone for the type-system revamp.
Related
- [Refactor]: Brand ErrorInstance so the type can be trusted at function boundaries #86 — branded
ErrorInstance. The class refactor obsoletes parts of [Refactor]: Brand ErrorInstance so the type can be trusted at function boundaries #86 (the helper, the cast) but keeps the brand declaration. - feat(errors): brand ErrorInstance so the type can be trusted at boundaries #87 — implementation of [Refactor]: Brand ErrorInstance so the type can be trusted at function boundaries #86. Should be re-evaluated after this issue lands.
- [BUG] is() should return TypeScript type predicate for narrowing #35 —
is()predicate. Lands naturally after the class because the brand check becomes a class check. - rules/0014-functions-over-classes-for-public-api.md — the rule that constrains the class to be internal. The class must be non-exported.
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
codewizdave commentedon Aug 12, 2026
@martyy-code test
codewizdave commentedon Aug 12, 2026
@marty-action test
codewizdave commentedon Aug 12, 2026
@marty-action test
marty-action commentedon Aug 12, 2026
It looks like the message you sent just says "test" — there's no specific task or question attached to this GitHub event.
Could you clarify what you'd like me to do? For reference, the GitHub context points to:
deessejs/errorscodewizdave)A few possibilities I can help with:
Please let me know which direction you'd like to go, or paste the actual request you intended.
codewizdave commentedon Aug 12, 2026
@martyy-code hey
marty-action commentedon Aug 12, 2026
Still just receiving "hey" with no actual task attached to the GitHub event for issue #88 in
deessejs/errors.To recap what's still on my plate from your side:
codewizdave)Happy to help once you let me know what you want. Some options:
Which one would you like?
refactor(errors): move ErrorInstance implementation to a private class (