Repository navigation
[Refactor]: Move ErrorInstance implementation to an internal class while keeping the FP public API #88
Copy link
Copy link
Closed
Labels
p2: mediumNormal priorityNormal prioritytype: refactorRefactoring / code restructuringRefactoring / code restructuring
Milestone
Description
Activity
- addedp2: mediumNormal priorityNormal prioritytype: refactorRefactoring / code restructuringRefactoring / code restructuring
on Aug 12, 2026 @martyy-code test
@marty-action test
@marty-action test
Reacted by marty-actionmarty-action commented
on Aug 12, 2026 marty-actionboton Aug 12, 2026 – with marty-actionMore actionsIt 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:
- Repository:
deessejs/errors - Issue: [Refactor]: Move ErrorInstance implementation to an internal class while keeping the FP public API #88 (with comment #5266766053 from
codewizdave)
A few possibilities I can help with:
- Read issue [Refactor]: Move ErrorInstance implementation to an internal class while keeping the FP public API #88 (or its comments) and summarize what it's about.
- Triage issue [Refactor]: Move ErrorInstance implementation to an internal class while keeping the FP public API #88 — fetch details, check related PRs/commits, and suggest next steps.
- Comment on the issue with a reply (please tell me what you want the comment to say — and note that per my instructions, I'll summarize and check with you before posting).
- Investigate a bug or PR related to the issue.
Please let me know which direction you'd like to go, or paste the actual request you intended.
- Repository:
@martyy-code hey
marty-action commented
on Aug 12, 2026 marty-actionboton Aug 12, 2026 – with marty-actionMore actionsStill 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:
- Issue: #88 (latest comment #5266784263 from
codewizdave) - Your message: "hey"
Happy to help once you let me know what you want. Some options:
- Read & summarize issue [Refactor]: Move ErrorInstance implementation to an internal class while keeping the FP public API #88 so you can see what's being discussed.
- Reply on the issue — tell me what you want the comment to say and I'll confirm before posting.
- Look for related PRs/commits to see if a fix is already in progress.
- Do something else entirely — just describe it.
Which one would you like?
- Issue: #88 (latest comment #5266784263 from
- added a commit that references this issue
on Oct 7, 2026
Metadata
Metadata
Assignees
Labels
p2: mediumNormal priorityNormal prioritytype: refactorRefactoring / code restructuringRefactoring / code restructuring
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:FACTORY_SYMBOLalready provides.new Error()+ cast + helper pattern. Pure refactor; preserves the FP API.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:
ErrorInstanceImpl<TFields>) owns the methods (from,addNote), the mutable state (notes,causes,context), and the brand assignment.error()instantiates the class internally, binds the public-facing methods, and returns a typed object whose type is exposed but whose class is not.error,is,causes,raise) is unchanged: factory functions return typed values, nonewis exposed.This would resolve several smells from the current shape:
brandInstance(instance)) becomes the class constructor's job. The cast on the brand property disappears.addNote/frommethods, currently attached viainstance.addNote = ...with no type narrowing, are real class methods with properthistypes.ErrorInstancethat lacks the brand.export class" remains satisfied: the class is internal to the module; only the factory function is exported.Trade-offs
Easier:
ascast at the construction site.thistypes instead of capturinginstanceviaconst.Harder:
export class ErrorInstanceImplfor "convenience" would violate the rule. The naming convention (Impl suffix, never exported) is the guard.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:
ErrorInstance<T>. Pure addition.is()predicate (issue [BUG] is() should return TypeScript type predicate for narrowing #35). Brand becomes reachable to consumers.The class-based refactor would sit at position 0 of the stack, before #87. It would obsolete the
brandInstancehelper (the constructor sets the brand) and theascast (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:
error()to a class internally, ship as a separate PR (this issue).brandInstancehelper dies.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.)Revisit conditions
This issue should be revisited when:
Symbol.hasInstancecustomisation).Effort and priority
m - 1-2 days. The class is straightforward; the tests pass unchanged.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.arch-rules-auditif bundled with the brand work; otherwise a new milestone for the type-system revamp.Related
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.is()predicate. Lands naturally after the class because the brand check becomes a class check.Pre-Submission Checklist