Skip to content

fix: address mapProps review points (#1-#4, B1, B2)#103

Merged
AlexandrHoroshih merged 1 commit into
masterfrom
followup/maprops-review-points
Jun 25, 2026
Merged

fix: address mapProps review points (#1-#4, B1, B2)#103
AlexandrHoroshih merged 1 commit into
masterfrom
followup/maprops-review-points

Conversation

@AlexandrHoroshih

Copy link
Copy Markdown
Member

Follow-up to PR #102 (mapProps feature). Addresses all review points:

#1 — Runtime tests added for variant, list, createReflect (no-ssr + ssr).
Previously only reflect had mapProps coverage.
#2 — Unknown mapProps keys now error at the key site itself, not on fn's
return. MapPropsFromSources resolves unknown-key entries to never,
producing a TS2322 at the key line. Replaces the old
'K extends keyof Props ? Props[K] : never' fn-return approach that
had a silent-failure path (never-returning fn compiled cleanly).
#3 — Documented the Store variable-source widening limitation.
Type tests (3a positive, 3b widening) pin the current behavior.
#4 — fn is skipped when its key is overridden by an external prop.
'if (key in props) continue' in src/core/reflect.ts. Spy assertions
in reflect and createReflect tests verify fn is not called.
B1 — bind + mapProps key collision is now a type error. MapPropsFromSources
takes Bind as a type parameter; a key in both bind and mapProps
resolves to never. Runtime skip ('if (key in storeProps) continue')
is a defense-in-depth for JS/type-bypass scenarios (covers stores;
events/data/functions are covered by the type fix).
B2 — mapItem + mapProps key collision in list is a type error. MapItem's
mapped type now omits keyof Sources alongside keyof Bind. Partial:
bypassable with explicit item-parameter annotation (known TS
limitation with mapped-type extends constraints), documented in the
type-test comment.

All four operators (reflect, createReflect, list, variant) are covered by the type fixes and runtime tests.

Docs updated in docs/pages/docs/reflect.mdx:

  • mapProps keys must be props of the view; unknown keys error at the key
  • a key must not appear in both bind and mapProps
  • in list, a key must not appear in both mapItem and mapProps
  • source should be an inline literal for best inference
  • fn is not invoked when its key is overridden

80 runtime tests + type tests pass.

Follow-up to PR #102 (mapProps feature). Addresses all review points:

#1 — Runtime tests added for variant, list, createReflect (no-ssr + ssr).
      Previously only reflect had mapProps coverage.
#2 — Unknown mapProps keys now error at the key site itself, not on fn's
      return. MapPropsFromSources resolves unknown-key entries to never,
      producing a TS2322 at the key line. Replaces the old
      'K extends keyof Props ? Props[K] : never' fn-return approach that
      had a silent-failure path (never-returning fn compiled cleanly).
#3 — Documented the Store<any> variable-source widening limitation.
      Type tests (3a positive, 3b widening) pin the current behavior.
#4 — fn is skipped when its key is overridden by an external prop.
      'if (key in props) continue' in src/core/reflect.ts. Spy assertions
      in reflect and createReflect tests verify fn is not called.
B1 — bind + mapProps key collision is now a type error. MapPropsFromSources
      takes Bind as a type parameter; a key in both bind and mapProps
      resolves to never. Runtime skip ('if (key in storeProps) continue')
      is a defense-in-depth for JS/type-bypass scenarios (covers stores;
      events/data/functions are covered by the type fix).
B2 — mapItem + mapProps key collision in list is a type error. MapItem's
      mapped type now omits keyof Sources alongside keyof Bind. Partial:
      bypassable with explicit item-parameter annotation (known TS
      limitation with mapped-type extends constraints), documented in the
      type-test comment.

All four operators (reflect, createReflect, list, variant) are covered
by the type fixes and runtime tests.

Docs updated in docs/pages/docs/reflect.mdx:
- mapProps keys must be props of the view; unknown keys error at the key
- a key must not appear in both bind and mapProps
- in list, a key must not appear in both mapItem and mapProps
- source should be an inline literal for best inference
- fn is not invoked when its key is overridden

80 runtime tests + type tests pass.
@bolt-new-by-stackblitz

Copy link
Copy Markdown

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@AlexandrHoroshih AlexandrHoroshih merged commit cd9f418 into master Jun 25, 2026
1 check passed
@AlexandrHoroshih AlexandrHoroshih deleted the followup/maprops-review-points branch June 25, 2026 12:36
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