✨ Leave out a prop whose expression produced nothing (#537) - #541
Conversation
PR #541: ✨ Leave out a prop whose expression produced nothing (#537)10 files, +477 / -45 Scope🟡 522 lines changed. PRs under 400 receive more thorough review. StructuralOxlint structural signals:
Slop
Oxlint slop signals:
Static AnalysisOxlint: 43 diagnostics across 2 files (9 rules) no-unnecessary-type-assertion (18): packages/core/src/scanner.ts, packages/core/src/expand.ts CorrectnessNo extraneous code patterns detected. |
An ordinary component's expression prop that evaluates successfully to
`undefined` is now absent rather than rejected: the omission happens in the
one resolver both Markdown and function components pass through, before
validation, so an optional prop stays unset, a declared default applies, and a
required one fails as missing with no body run. `null`, evaluation failures,
non-serializable roots, nested normalization, captures and `<Let value>` are
unchanged, and nothing durable holds `undefined`.
The scanner keeps an ordinary component's literal `{undefined}` as an
expression so it reaches that same resolver; a construct's arbitrary-value
operand keeps its scan-time reading.
One authored `<PullRequest number={pullRequest.number}>` inside a bounded loop
now creates without a number and then updates the number it created.
6681492 to
7d2e1c5
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // expansion is where that is decided (§6.5). Reading the literal here | ||
| // would answer with `null` before the prop ever reaches it. A construct | ||
| // evaluates its own operand under its own contract, so what it is | ||
| // handed at scan time is unchanged. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
| // handed at scan time is unchanged. |
1 participant
Why
An author cannot write one component invocation whose optional identity prop is
absent until a preceding result supplies it.
number={pullRequest.number}failsthe whole document on the first iteration, because a successful
undefinedisrejected at the prop JSON boundary — so the same element has to be written
twice, once for each case. Closes #537.
What changes
An ordinary component's expression prop that evaluates successfully to
undefinedis omitted.Before:
After:
numberis simply not there. The schema answers the absence — anoptional prop stays unset, a declared
defaultapplies, and a required onefails as a missing property with no body run — so the first iteration asks for a
pull request to exist and the next names the one it created.
How it works
Both component kinds already pass through
resolveExpressionProps()beforevalidation, so the omission is decided once and neither kind gets a rule of its
own. The scanner change is what makes a literal
{undefined}reach thatresolver at all: it used to be read as JSON at scan time, which answered the
question with
nullbefore the boundary existed. Only a non-structuralcomponent's prop is affected — a construct evaluates its own operand under its
own contract and keeps its scan-time reading.
Review guide
Start with:
specs/executable-mdx-spec.md§6.5, "A successfulundefinedis an absence"
Then review:
architecture.md→## The ordinary component prop boundaryresolveExpressionProps()inpackages/core/src/expand.tsparseAttributes()inpackages/core/src/scanner.tspackages/core/tests/expression-props.test.ts, and theworkflow PullRequest across a loopsuiteLook carefully at:
validateProps(), not after: deleting the member laterwould prevent defaults and make a required member look satisfied.
typeof result === "undefined"is omission. A function, a symbol and aserialization that threw are still the failure they were.
What must stay true
nullis a value, not an absence — enforced by leavingnullon the JSONround trip; checked by
EU5andJ7b.recursive walker; checked by
EU7andJ2.<Let value>still receive the exact operand — enforced bycapture separation running before the resolver and by
bindsByReference();checked by
CP1,CP8,LET2–LET3,J7.isStructural()guard in the scanner; checked byA-REF2andIF14.undefined— enforced by omitting before anythingis written and by adding no record for prop resolution; checked by
EU8.How to verify it
EU1/EU2prove a runtime member and a literal take the same path, and failif the scanner's old
nullprojection bypasses the resolver.EU3/EU4prove ordering: a default is supplied and a required prop fails asmissing with zero body runs. They fail if omission happens after validation.
EU8proves the journal holds noundefinedand no record of propresolution, that a partial replay reconstructs the same props through ordinary
expansion, and that a completed replay reuses its terminal result and imports
nothing again.
CR22cproves an overriding repository component crosses the ordinaryboundary while core's capturing
<Json>does not.creates without a number, then updates the number it createddrives the realcomponent, run database, DOFS Workspace, local Git remote and fake GitHub:
exactly one creation from a normalized
number: nulland a create naturalkey, then one
patchunder the created number with an update natural key.Focused commands, all green:
Two mutation runs confirm the coverage discriminates: restoring the old
rejection fails
EU1–EU5,EU8,CP8,CR22cand the PullRequest looptest; disabling only the scanner branch fails
A-REF4,EU2andCP8.Scope
Included
undefinedomits an ordinary expression prop, beforevalidation and before the JSON boundary.
{undefined}as anexpression.
architecture.mdinvariant, the §6.5 contract, the scanner/EU/CR testmatrix rows and decision 102.
<PullRequest>consumer integration through the shipped provider.Intentionally unchanged
packages/workflow/src/composition/components/PullRequest.ts. Its schemaalready makes
numberoptional and it already normalizes absence to thedurable
nullthe create request uses. Teaching the provider aboutundefinedwould hide the language defect behind a second contract.null, evaluation failures, unbound names, non-serializable roots, nestedJSON.stringifynormalization, captures,<Let value>,<Json value>, andevery structural operand contract.
middleware or public API, and no dependency or lockfile change.
Risks and limitations
<Loop max={…}>and<Each in={…}>resolve their one prop through the sameshared resolver. A runtime expression producing
undefinedthere used to failas "non-serializable" and now fails with the construct's own message (an
invalid bound, "must resolve to an array"). Both are failures and no test
asserted the old wording.
as={undefined}on an ordinary component is now refused as "must be a stringliteral" rather than through the binding-name check on
null— the reservedasrule governs it either way.authoredExpressionsmay no longer have an observable discriminator:{undefined}was the only JSON-readable text whose reading and evaluationdiffered, and it is now an expression. Left in place; noted for the Planner.
Scope confirmation