fix(schema): check defineField options against the field type - #753
Open
maoberlehner wants to merge 7 commits into
Open
fix(schema): check defineField options against the field type#753maoberlehner wants to merge 7 commits into
maoberlehner wants to merge 7 commits into
Conversation
maoberlehner
force-pushed
the
fix/schema-define-field-excess-property-checks
branch
from
August 13, 2026 10:33
20aac04 to
4ed6407
Compare
@storyblok/angular
@storyblok/astro
@storyblok/api-client
storyblok
@storyblok/experiments
@storyblok/js
storyblok-js-client
@storyblok/lint-config
@storyblok/live-preview
@storyblok/management-api-client
@storyblok/migrations
@storyblok/nuxt
@storyblok/react
@storyblok/region-helper
@storyblok/richtext
@storyblok/schema
@storyblok/svelte
@storyblok/vue
commit: |
Contributor
|
maoberlehner
marked this pull request as ready for review
August 13, 2026 12:25
maoberlehner
force-pushed
the
fix/schema-define-field-excess-property-checks
branch
from
August 13, 2026 15:21
58c7282 to
d7d1c12
Compare
maoberlehner
force-pushed
the
fix/schema-define-field-excess-property-checks
branch
from
August 14, 2026 08:55
d7d1c12 to
f9d7d12
Compare
maoberlehner
force-pushed
the
fix/schema-define-field-excess-property-checks
branch
from
August 14, 2026 09:17
f9d7d12 to
a0fb628
Compare
maoberlehner
force-pushed
the
fix/schema-define-field-excess-property-checks
branch
from
August 14, 2026 09:46
a0fb628 to
d18a162
Compare
`defineField` accepted any key. Because the field literal is inferred as the
type parameter, the only check was plain assignability to the constraint, which
ignores excess properties. Typos and options belonging to a different field type
compiled clean, were pushed to the Management API verbatim, and silently did
nothing:
defineField('x', { type: 'bloks', component_group_whitlist: ['a'] });
defineField('x', { type: 'text', component_group_whitelist: ['x'] });
defineField('x', { type: 'asset', restrict_components: true });
Intersect the parameter with `NoExtraKeys<T>`, a mapped type over the keys the
matched `Field` variant does not own, to give excess property checking the
concrete target it needs. `type: 'custom'` is exempt, since plugin option keys
pass through to the Management API verbatim.
Separately, `allow`/`deny` and the wire keys they derive were silently both
legal, and `mapFieldToWire` overwrites the hand-written value, so one of the two
was dropped without warning. `NoRestrictionConflict<T>` makes the combination a
compile error, and `validateSchema` reports the same conflict as a
`conflicting_restriction` issue for consumers without type checking. Both read
from one `DERIVED_RESTRICTION_KEYS` list so they cannot drift.
The five derived keys now carry `@deprecated` JSDoc pointing at `allow`/`deny`,
each stating why and what to use instead. `FieldInput` is built on a
distributive `Omit` over `Field` so it is their only declaration site:
TypeScript reports a property as deprecated only when every declaration carries
the tag, so inheriting the untagged `Field` declaration cancelled the tag out.
`restrict_type` and the two tag lists stay untagged, as `restrict_type: 'tags'`
is the one restriction dimension with no DSL equivalent.
Rejected keys resolve to `Invalid<TReason>`, an unsatisfiable `unique symbol`
branded type whose argument carries the reason into the compiler error.
Refs #750
`schema init` classified a tag-restricted field as `kind: "none"` — the branch for fields with no restriction in force, which drops `restrict_components` and `restrict_type` as byproducts `allow`/`deny` re-derive on push. Nothing re-derives them for the tag dimension: `allow`/`deny` cannot express tags at all. So the round-trip pushed back a field whose tag lists the editor never reads, silently restricting by block name instead. Classify a tag restriction as its own kind, keyed on `restrict_type` (the key that puts the lists in force), and keep its wire form flags included. Checked before the name and group dimensions so a stale list left behind by switching dimensions in the editor is dropped rather than emitted as `allow`, which would re-derive `restrict_type: ''` and move the same loss elsewhere.
`FieldInput` grafted all eight wire restriction keys onto every `Field` variant,
while `NoExtraKeys` checks a literal against the matched variant. So
`FieldInput`'s own `text` member carried `component_whitelist`, which `text` does
not own, and the exported type documented as "field config accepted by
`defineField`" failed that check:
const f: FieldInput = { type: 'text', max_length: 10 };
defineField('a', f); // TS2345
Build `FieldInput` one variant at a time instead, re-picking only the wire keys
that variant actually declares. Both sides now derive from the same variant, so
they cannot disagree. The `@deprecated` tags survive the `Pick` (verified through
the compiler API: still reported on the five derived keys of the `bloks` member,
still absent on the tag keys and `restrict_type`), and the keys stay rejected on
variants that do not own them, which is what the check was added for.
A generic passthrough wrapper (`<F extends FieldInput>(f: F) => defineField(n, f)`)
still needs a cast. That is inherent to intersection-based excess property
checking — `F` cannot be proven assignable to `F & NoExtraKeys<F>` while it is
unresolved — and is not addressed here.
`resolveFieldRestriction` only recognised a tag restriction when a tag list was
non-empty. A field with `restrict_type: 'tags'` and empty lists fell through to
the name dimension and then to `kind: "none"`, which drops `restrict_type` and
`restrict_components` as byproducts `allow`/`deny` re-derive on push — except
there is no `allow`/`deny` here to re-derive them.
Reproduced against a real space: `schema init` emitted a bare
defineField('content', { type: 'bloks' })
for a field the space stored as `restrict_components: true, restrict_type:
'tags'`, so the round-trip unrestricted the field.
Key the dimension on `restrict_type` alone, as the surrounding docs already
described. The non-empty guard was there to stop a stale name list left behind by
switching dimensions from being emitted as `allow`, but the Management API clears
the name and group lists when `restrict_type` is `'tags'` (verified against the
API: a `component_whitelist` written alongside `restrict_type: 'tags'` comes back
stripped), so there is no stale list to guard against.
Two related losses in `schema init`, both found by round-tripping a real space.
`restrict_components: true` with an empty `component_whitelist` was classified as
`kind: "none"` and dropped as a wire byproduct that `allow` re-derives on push.
But an empty list emits no `allow`, so nothing re-derived it and the round-trip
turned "restricted, nothing selected" into "unrestricted". Keep the flag, and the
dimension selector with it, when no list is in force.
Keeping it exposed a second problem. The Management API stores a component schema
as an opaque blob, so a space can hold restriction keys on a field type that does
not own them — a stray `restrict_components` on an `asset` field is stored
verbatim (verified against the API). `defineField` rejects an option the field
type does not own, so emitting those keys generates code that does not compile.
This was already reachable through the `disabled` branch before this change: a
space with `restrict_components: false` on an `asset` field made `schema init`
emit a file failing with
error TS2322: Type 'false' is not assignable to type
'false & Invalid<"unknown option "restrict_components" for this field type">'
Only emit the restriction keys for the field types that own them. That covers the
tag lists too, which reached the output through the untouched-key passthrough
rather than the restriction branches and so needed the same guard.
Verified over a 21-field matrix covering every restriction dimension and the full
editor option surface: the generated file type-checks, no field round-trips to a
*different* value, and a second push is a no-op. The only keys dropped are empty
lists and keys on field types that cannot read them.
The tag dimension claimed any field with `restrict_type: 'tags'`, including one carrying a live `component_whitelist`. The editor clears all six lists on a dimension switch and the Management API only backstops that for `bloks` fields, so a `richtext` can hold that pairing from legacy or API writes, and claiming it for a dimension that selects nothing dropped the only list in force. The tag branch now needs a non-empty tag list; a tag dimension with empty lists falls to `raw`, which keeps `restrict_type` and `restrict_components` verbatim as before. Also corrects three comments against editor-authored data: Management API normalization is `bloks`-only and strips name lists only, the editor and the backend genuinely disagree on an absent `restrict_components`, and allow plus deny in one dimension is not reachable through the editor.
The regression test added with the fix covered `text` and `bloks`. `bloks` legitimately owns the wire restriction keys, so it cannot fail the check — the guard was really one variant wide, while `FieldInput` is now assembled one variant at a time and a regression can hit any single one. Checked against the pre-fix tree: 14 of the 17 variants reproduce the original TS2345. The three that do not (`richtext`, `bloks`, `custom`) are the ones that declare the keys themselves. The annotation on the const is what makes this bite. Routing the same values through a `(f: FieldInput) => ...` parameter compiles clean even on the broken tree, so the consts are deliberate and noted as such.
maoberlehner
force-pushed
the
fix/schema-define-field-excess-property-checks
branch
from
August 14, 2026 10:01
d18a162 to
ea002c2
Compare
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.
Important
Stacked on #752 (
fix/schema-deny-leaks-to-wire) and targets that branch, notmain.Review that one first. This PR depends on it in two ways:
component_denylistcan only point atdenyoncedenyreaches the wire, andcomponent_group_denylist's bucket depended on thatbranch's folder-ref decision.
Two defects
1. No excess property checking.
defineFieldaccepted any key. The field literal is inferredas the type parameter, so the only check was plain assignability to the constraint, which ignores
excess properties. Typos and options meaningless for the field type compiled clean, were pushed to
the Management API verbatim, and silently did nothing:
2.
allow/denyand the raw wire keys were silently both legal.mapFieldToWirespreads...restand then assigns fromallow, so a hand-writtencomponent_whitelistwas overwrittenwithout warning. Setting both is always a mistake.
Approach
NoExtraKeys<T>gives excess property checking the concrete target it lacks, keyed off the matchedFieldvariant so the check is per field type.type: 'custom'is exempt, since plugin optionkeys legitimately pass through verbatim.
NoRestrictionConflict<T>makes mixingallow/denywith the five keys they derive a compileerror.
validateSchemareports the same conflict as aconflicting_restrictionissue forconsumers authoring schemas in plain JavaScript. Both read from one
DERIVED_RESTRICTION_KEYSlist, so they cannot drift.
Rejected keys resolve to
Invalid<TReason>, an unsatisfiableunique symbolbranded type whoseargument carries the reason into the error:
The wire keys stay legal as a lower-level escape hatch. They are not removed, and
restrict_components: falseandrestrict_type: 'tags'plus the tag lists still compile.Notes for review
@deprecatedcannot produce an inline strikethrough, and that is a TypeScript limitation. Probedwith the language service on TS 6.0.3 and 5.8.3:
order independent, unions included. TS requires every declaration to carry the tag. Hence
FieldWithoutRestrictions, a distributiveOmitoverField, soFieldInputis the onlydeclaration site. All five keys now report;
restrict_typeand the tag keys correctly do not.Confirmed across plain types, intersections, discriminated unions, and interfaces. So no type
design yields a strikethrough on a written
component_whitelist:key. What the tags do deliver:the struck-through autocomplete entry with its JSDoc example, a strikethrough on reads, and the
reason in the conflict error. An inline nudge on the written key would need a lint rule.
Follow-on:
schema initdropped the tag dimensionManual QA against a QA space surfaced a related bug in
packages/cli, fixed in thefix(cli)commits here since it lands in the same feature area and this PR's docs point straight at it.
resolveFieldRestrictionclassified a tag-restricted field askind: "none"— the branch forfields with no restriction in force, which drops
restrict_componentsandrestrict_typeasbyproducts
allow/denyre-derive on push. Nothing re-derives them for the tag dimension, becauseallow/denycannot express tags at all. Soschema init→schema pushpushed back a field whosetag lists the editor never reads, silently restricting by block name instead.
The fix classifies a tag restriction as its own kind, keyed on
restrict_typeand a non-emptytag list.
restrict_typealone is not enough: the editor clears all six lists on a dimension switch,and the Management API backstops that for
bloksfields only, so arichtextcan holdrestrict_type: 'tags'next to a livecomponent_whitelistfrom legacy or direct API writes. Theeditor reads the name list in exactly that case —
allowedComponents.tsgates every dimension on anon-empty list — so a tag dimension that selects something keeps its wire form with the flags intact,
while an empty one falls through to whichever list is actually in force.
Verified end to end against the
has-restrictionsscenario, which covers all nine restrictionflavors: the generated code typechecks clean under the stricter
defineField, no field round-tripsto a semantically different value, and a second push reports every entity
unchanged. The firstpush after
initis not a no-op on an editor-authored space — the editor always writes the pairedempty list, and
initdrops it — but every one of those removals is inert, since the editor gateseach list on
?.length. Regression tests cover the lossless round-trip, the stale-name-list case,and the gate.
Known gaps, out of scope here
schema pushnever callsvalidateSchema(onlyschema validatedoes), so the newconflicting_restrictiondiagnostic protects JS authors only if they runvalidatefirst. Thecompile-time rejection is the real guard.
restrict_typeis itself overwritten byallow/denyat push time (map-to-wire.ts), but isdeliberately not in
DERIVED_RESTRICTION_KEYS, so neither the type nor the validator flagsallow+restrict_type: 'tags'. The tag list ends up inert. Worth a follow-up.deprecations on reads only. The tags deliver a struck-through autocomplete entry with its reason
and example, and nothing after that. Catching a written key would need a lint rule.
Fixes DX-550