Skip to content

fix(publisher): repair invalid props one by one instead of resetting the node - #574

Open
tommy230 wants to merge 1 commit into
CoreBunch:mainfrom
tommy230:fix/validate-node-props-per-prop
Open

tommy230 wants to merge 1 commit into
CoreBunch:mainfrom
tommy230:fix/validate-node-props-per-prop

Conversation

@tommy230

@tommy230 tommy230 commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

When a single prop on a node fails its TypeBox schema at publish time, the whole node is published with module defaults. A link whose stored target is a value the schema does not model (for example "" from an HTML import, or a value left behind after a schema was tightened) renders as "Click here" pointing at #, even though its href and text are valid and still in the store. Nothing fails, so the only way to notice is to read the published page.

<!-- stored props: { href: "/contact/", text: "Contact us", target: "" } -->

<!-- published today -->
<a href="#" target="_self">Click here</a>

<!-- published with this change -->
<a href="/contact/" target="_self">Contact us</a>

The cause is the catch branch of validateNodeProps in src/core/module-engine/validateNodeProps.ts: when Value.Parse throws for the node, it returns { ...rawProps, ...def.defaults }, which overwrites every declared prop, not just the one that failed.

The fix:

  • When whole-node parsing fails and the props schema is an object, each declared prop is parsed on its own (default, convert, check for that leaf). Props that pass stay as authored; only a prop that still fails takes the module default for that key.
  • Absent optional props stay absent unless their schema declares a default, in which case they are filled as on the successful path. Injected unknown keys still survive.
  • A non-object props schema, or one without TypeBox's Kind symbol (for example a plugin schema rebuilt from JSON), keeps the old wholesale fallback.
  • A failing prop with no module default keeps its authored value, as it did before.
  • The fast path and the successful slow path are untouched; the new code runs only where the old fallback ran.

Trade-off: a node with one broken prop now publishes with the rest of its content instead of a visibly default block. That fits the boundary's stated purpose (normalise "stale, missing, or lightly malformed" props before render), but it makes a bad stored value quieter, not louder. I did not add a dev warning: the publisher has no logging or warnings channel today and render() runs once per node per publish, so a console.warn here would be noise on large sites. If you would rather surface these repairs, a count on the publish result is the place, and I am happy to add it.

#575 stops the HTML importer from storing unsupported target values in the first place; this PR covers nodes that already carry one, or any other single bad prop.

Verification

  • bun run build
  • bun run lint
  • bun test: 7062 pass, 0 fail
  • Docker/deployment check, if relevant: not relevant

New tests in src/__tests__/module-engine/validateNodeProps.test.ts: 5 of 9 fail on main and pass with this change. The non-throwing check also passes on main, which spread undefined defaults harmlessly.

Checklist

  • Tests cover behavior changes.
  • Docs were updated when behavior, config, deployment, or public surfaces changed. No documented behavior changed; the fallback is described in the file header comment.
  • No compatibility shim was added for old pre-release behavior.
  • No secrets, local databases, uploads, or generated artifacts are included.

🤖 Generated with Claude Code

…the node

When Value.Parse rejected a node's props, validateNodeProps replaced every
declared prop with the module default, so a link with one unsupported
target published as "Click here" pointing at "#" while its href and text
sat valid in the store. The catch branch now parses each declared prop on
its own and falls back to the default only for the props that still fail.
Fast path, successful slow path, optional props and injected keys are
unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tommy230
tommy230 force-pushed the fix/validate-node-props-per-prop branch from 9d48f30 to 8b18ab6 Compare September 29, 2026 00:02
@tommy230
tommy230 marked this pull request as ready for review September 29, 2026 04:47

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant