feat!: forbid subclassing an entity class - #8
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #2, which made subclassing correct. This makes it illegal, so
that case can no longer arise and
instance.tsstays atmain's simplerself-overwriting getter.
The rule
One
extendsis the declaration form. Anything deeper defects:Using the builder's return directly, without
extends, is unaffected — bothare covered by tests.
Why a
Defect, not a throw or anInvalidEntityA bare
throwin the constructor would escapedecode()and break theResultcontract. It routes throughfromThrowable(..., (cause, defect) => defect(cause))instead — unthrownexposes no direct
Defectconstructor, and this is the repo's sanctioneddefect path. Not
InvalidEntity, because this is a bug in domain code ratherthan bad caller input, which is the same line the existing
add-output defectdraws.
Side benefit: constructor throws in general now become defects instead of
escaping.
TypeError: Cannot redefine property— what you get if a subclassredeclares a data field — was previously thrown straight out of
decode().Runtime only
TypeScript has no
final, and aprivate/protectedconstructor cannotexpress "extendable once". Both measured:
private→TS2675: Cannot extend a class 'Base'— the declaration formstops compiling.
protected→TS2674(seals, good) butTS2684: Cannot assign a 'protected' constructor type to a 'public' constructor type— the statics stop returningthe subclass.
So
class Sub extends Organization {}compiles and reports on firstconstruction. Closing that gap needs a lint rule; see below.
Fallout
equals: the sibling-subclass rule documented in chore: cover the published Node floor in CI and record two invisible couplings #6 is removed — twosubclasses of one entity can no longer both exist.
OrgWithCacheexamples in the README and tests move from a subclass to afirst-level class body, which is the pattern being steered to anyway. What
they pin is unchanged: class-body fields stay writable (why
Object.freeze(this)cannot be used) and are absent fromtoJSON().CLAUDE.mdconvention entry.Not done: the oxlint rule
A lint rule is what would make this compile-time. Deliberately not in this PR:
detecting
class Sub extends SomeEntity {}across files needs typeinformation, and I have not confirmed oxlint's JS plugin API exposes a
checker. Same-file cases are trivially syntactic. It also belongs in
@btravstack/oxlint, since it is a rule about consuming this library.Gate
format --check,lint,typecheck(both passes),test(90, 9 files),knip,build— all green.