docs: correct snippets and pointers left by the Diataxis split - #23
Merged
Conversation
Review of #22 turned up eight defects, all introduced or left behind by the restructure. Three would fail to compile if copied: - `http-contract.md` called `converter.convert(schema, { strategy: "input" })`. `@orpc/zod`'s converter takes the direction as a bare string, as the type-checked `contract.spec.ts` already spells it, and returns a `[jsonSchema, optional]` tuple. - `model-an-aggregate.md` read `order.customer.shout` from a `Customer` declared without the `computed` option the source spec gives it. Restore the option rather than delete the usage; the round-trip comment depends on it. - `persist-and-rehydrate.md` used the schema value `OrgId` in type position. `Entity.factory` / `factoryAsync` / `make` are statics on a declared entity, not on the `Entity` builder, which carries only `union`. Both the reference and the README table spelled them `Entity.*`, indistinguishable from the real `Entity.union` two sections away. Use `SomeEntity.*`, as `extend` already did. The test-domain-logic id generator interpolated a counter into a fixed run of zeros, so the last UUID group grew to 13 characters on the tenth call and the factory started returning `Err(InvalidEntity)` for a reason unrelated to the test. Repoint two references the split invalidated: CONTRIBUTING's design-rules section and `freeze.ts`'s docstring both pointed at README sections that now live in `docs/`. Finally the changeset claimed nothing was discarded, which was not true and would have shipped verbatim to the CHANGELOG. Restore what was missing -- the computed-vs-getter rule, the immutability compile-error examples, the `factoryAsync` example -- and drop the claim, which covers more than was verified. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR corrects documentation snippets, references, and pointers that were left incorrect after the Diátaxis documentation split (with no runtime behavior changes; only a docstring pointer in src/).
Changes:
- Fix multiple docs snippets so they reflect the real API surface and compile when used as intended in the guides.
- Clarify entry-point usage by switching misleading
Entity.*references toSomeEntity.*in docs/README where appropriate. - Repair pointers invalidated by the split (CONTRIBUTING +
freeze.tsdocstring) and remove an inaccurate changeset claim.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates entry-point table to use SomeEntity.* instead of Entity.*. |
| packages/entity/src/freeze.ts | Updates the docstring pointer from README to docs/explanation.md. |
| docs/reference.md | Renames Entity.* headings to SomeEntity.* and restores/expands factoryAsync example. |
| docs/how-to/test-domain-logic.md | Fixes the sample UUID generator to remain valid past 9 calls. |
| docs/how-to/persist-and-rehydrate.md | Fixes OrgId type usage in the repository signature (z.infer<typeof OrgId>). |
| docs/how-to/model-an-aggregate.md | Restores the computed option so the example matches its later usage. |
| docs/how-to/http-contract.md | Fixes @orpc/zod converter usage (direction string + tuple return). |
| docs/explanation.md | Restores explanation/examples around deep immutability and computed-vs-getter guidance. |
| CONTRIBUTING.md | Repoints “Design rules” documentation references to the new docs locations. |
| .changeset/docs-diataxis.md | Removes the inaccurate “Nothing was discarded” claim from the release text. |
Suppressed comments (2)
docs/reference.md:89
- Same issue as
factory: the heading listsAsyncResult<Entity, InvalidEntity>, but the API returnsAsyncResult<T, InvalidEntity>for the concrete entity class. UsingSomeEntityin the return type matches the actual typing and avoids conflating the return type with theEntitybuilder.
### `SomeEntity.factoryAsync(generators)` → `(input) => AsyncResult<Entity, InvalidEntity>`
docs/reference.md:103
makereturns aResult<T, InvalidEntity>for the concrete declared entity class, notResult<Entity, InvalidEntity>. Updating the heading avoids implying there is a runtime/type namedEntityreturned here.
### `SomeEntity.make(data)` → `Result<Entity, InvalidEntity>`
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The `Entity.*` -> `SomeEntity.*` pass renamed the headings but left `Result<Entity, InvalidEntity>` in the return position, which is worse than before: this PR gave `Entity` the specific meaning "the builder", and the builder is not what comes back. `make<T>`, `factory<T>` and `factoryAsync<T>` are all `this`-typed on the concrete class, so the result is that class. `entity.update` had the same defect and is fixed with them. Also drop the quoted engine message from the frozen-array example. V8 says "Cannot add property 1, object is not extensible" rather than the string the old README carried, and other engines word it differently again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
A review of #22 turned up eight defects, all introduced or left behind by the restructure. No runtime behaviour changes; the only
src/edit is a docstring pointer.Snippets that would not compile if copied
docs/how-to/http-contract.mdcalledconverter.convert(schema, { strategy: "input" }).@orpc/zod's converter takes the direction as a bare string — as the type-checkedpackages/entity/src/contract.spec.ts:28already spells it — and returns a[jsonSchema, optional]tuple, which the snippet now destructures.docs/how-to/model-an-aggregate.mdreadorder.customer.shoutfrom aCustomerdeclared without thecomputedoption. The example came fromnesting.spec.ts, whereCustomerdoes declare it; the restructure dropped the options object and kept the usage. Restored the option rather than deleting the usage — theJSON.stringifyround-trip comment depends on it.docs/how-to/persist-and-rehydrate.mdused the schema valueOrgIdin type position. Nowz.infer<typeof OrgId>, as the other guides spell it.Entity.*vsSomeEntity.*factory,factoryAsyncandmakeare statics on a declared entity class; the exportedEntitybuilder carries onlyunion. Bothdocs/reference.mdand the README entry-point table spelled themEntity.*, indistinguishable from the genuineEntity.uniontwo sections away — soEntity.make(row)reads as valid and fails withTypeError: Entity.make is not a function. NowSomeEntity.*, matching the spellingextendalready used.An id generator that breaks on the tenth call
docs/how-to/test-domain-logic.mdbuilt`…00000000000${(n += 1)}`. The last UUID group is 12 hex characters only whilen <= 9; on the tenthcreateOrg(...)the group becomes 13,z.uuid()rejects, and the factory returnsErr(InvalidEntity)— so a downstream.getOrThrow()fails for a reason unrelated to what the test is exercising. NowString((n += 1)).padStart(12, "0").Pointers the split invalidated
CONTRIBUTING.md's binding "Design rules" section pointed atpackages/entity/README.mdfor the public behaviour; that file is now a short npm card. Repointed atdocs/reference.mdanddocs/explanation.md.packages/entity/src/freeze.ts's docstring ended "the README says so" about the live-mutable-object carve-out. That statement now lives atdocs/explanation.md. Per the repo convention that these comments are load-bearing pointers rather than decoration, repointed.The changeset claim
.changeset/docs-diataxis.mdasserted "Nothing was discarded", which would have shipped verbatim into the CHANGELOG and was not accurate. Rather than only soften it, this restores the three things the review identified as lost:docs/explanation.mdTypeErrorexamples →docs/explanation.mdfactoryAsyncusage example →docs/reference.mdThe sentence is still dropped: the restored sections are what was verified missing, and the claim covered more than that.
Verification
Full gate green —
format --check,lint,typecheck(all three passes),test(120 passed),knip,build.The gate does not type-check documentation snippets, so the changed ones were checked separately: a scratch module under
packages/entity/srcreproducing the aggregate declaration and usage, both converter calls, the repository signature, and the id generator compiles clean under the project'stsc. The scratch file is not part of this branch.Not changed
The how-to guides reference branded schemas (
OrgId,Slug,CustomerId,Upper, …) that no snippet defines — the preamble blockquotes cover imports only. It is consistent across all four guides and reads as deliberate shorthand, so it is left alone here, but it does mean "make every snippet's imports explicit" (08a98a2) stops short of the domain vocabulary. Worth a follow-up if the intent was fully copy-pasteable examples.🤖 Generated with Claude Code