diff --git a/docs/pages/docs/reflect.mdx b/docs/pages/docs/reflect.mdx index 4e2dd5e..18761b3 100644 --- a/docs/pages/docs/reflect.mdx +++ b/docs/pages/docs/reflect.mdx @@ -95,6 +95,8 @@ Each entry is `{ source, fn }`: - an **array** of stores — `value` is the resolved tuple (`[Store, Store]` → `[A, B]`). - `fn` — `(value, props) => derivedProp`, where `value` is the resolved `source` value (its type is inferred — no annotation needed) and `props` are the component's own props. +> Each key in `mapProps` must be a prop of the `view` — a typo'd or unknown key is a type error at the key itself. A key must not appear in both `bind` and `mapProps` — this is a type error. For best type inference, pass `source` as an inline literal; a variable typed as `Store` will widen `fn`'s `value` argument to `any`. + ```tsx import { reflect } from '@effector/reflect'; import { createStore } from 'effector'; @@ -143,9 +145,10 @@ const Hello = reflect({ The component re-renders only when the `source` changes. A prop computed via `mapProps` is made **optional** in the resulting component's type and can still be overridden explicitly at -the usage site (an explicitly passed prop wins over the derived value). +the usage site (an explicitly passed prop wins over the derived value — in that case `fn` is +not invoked for the overridden key). -> Note: like `bind`, the `mapProps` field is supported by all Reflect operators — `reflect`, `createReflect`, `variant` and `list`. +> Note: like `bind`, the `mapProps` field is supported by all Reflect operators — `reflect`, `createReflect`, `variant` and `list`. In `list`, a key must not appear in both `mapItem` and `mapProps` — `mapItem` automatically omits keys that are derived via `mapProps`. ### Fork API auto-compatibility diff --git a/public-types/reflect.d.ts b/public-types/reflect.d.ts index b748c81..33c2f07 100644 --- a/public-types/reflect.d.ts +++ b/public-types/reflect.d.ts @@ -75,16 +75,23 @@ type SourceValue = S extends Store * `Sources` is a separate generic that captures the `source` of every entry. Because the * sources are inferred independently of the `fn`s, `fn`'s `value` argument is inferred as the * resolved source value (no manual annotation needed), and `props` is the view's `Props`. - * `fn` must return the prop type - a key that is not a prop of the view resolves to `never`. + * Each key in `mapProps` must be a prop of the `view` - a typo'd or unknown key resolves its + * entry to `never`, so the object literal assigned to it is a type error at the key site. + * A key that is also present in `bind` resolves to `never` as well — a prop must not be + * both bound and derived. */ -type MapPropsFromSources> = { - [K in keyof Sources]: { - source: Sources[K]; - fn: ( - value: SourceValue, - props: Props, - ) => K extends keyof Props ? Props[K] : never; - }; +type MapPropsFromSources> = { + [K in keyof Sources]: K extends keyof Props + ? K extends keyof Bind + ? never + : { + source: Sources[K]; + fn: ( + value: SourceValue, + props: Props, + ) => Props[K & keyof Props]; + } + : never; }; /** @@ -144,7 +151,7 @@ export function reflect< /** * Derives props for the `view` from a store value combined with the component's props. */ - mapProps?: MapPropsFromSources; + mapProps?: MapPropsFromSources; hooks?: Hooks; /** * This configuration is passed directly to `useUnit`'s hook second argument. @@ -187,7 +194,7 @@ export function createReflect< /** * Derives props for the `view` from a store value combined with the component's props. */ - mapProps?: MapPropsFromSources; + mapProps?: MapPropsFromSources; hooks?: Hooks; /** * This configuration is passed directly to `useUnit`'s hook second argument. @@ -226,7 +233,10 @@ export function list< Props extends ComponentProps, Item, MapItem extends { - [M in keyof Omit]: (item: Item, index: number) => Props[M]; + [M in keyof Omit]: ( + item: Item, + index: number, + ) => Props[M]; }, Bind extends BindFromProps = object, // eslint-disable-next-line @typescript-eslint/ban-types @@ -238,7 +248,7 @@ export function list< view: View; bind?: Bind; mapItem?: MapItem; - mapProps?: MapPropsFromSources; + mapProps?: MapPropsFromSources; getKey?: (item: Item) => React.Key; hooks?: Hooks; /** @@ -251,7 +261,7 @@ export function list< view: View; bind?: Bind; mapItem: MapItem; - mapProps?: MapPropsFromSources; + mapProps?: MapPropsFromSources; getKey?: (item: Item) => React.Key; hooks?: Hooks; /** @@ -321,7 +331,7 @@ export function variant< cases: Partial; default?: ComponentType; bind?: Bind; - mapProps?: MapPropsFromSources; + mapProps?: MapPropsFromSources; hooks?: Hooks; /** * This configuration is passed directly to `useUnit`'s hook second argument. @@ -333,7 +343,7 @@ export function variant< then: ComponentType; else?: ComponentType; bind?: Bind; - mapProps?: MapPropsFromSources; + mapProps?: MapPropsFromSources; hooks?: Hooks; /** * This configuration is passed directly to `useUnit`'s hook second argument. diff --git a/src/core/reflect.ts b/src/core/reflect.ts index da0f41d..00bb1e9 100644 --- a/src/core/reflect.ts +++ b/src/core/reflect.ts @@ -79,6 +79,9 @@ export function reflectFactory(context: Context) { ); for (const key of mapPropsKeys) { + // an explicitly passed prop or a bound store wins over the derived one — skip fn + if (key in props) continue; + if (key in storeProps) continue; mappedProps[key] = (mapProps as any)[key].fn( (mapPropsValues as any)[key], propsForFn, diff --git a/src/no-ssr/create-reflect.test.tsx b/src/no-ssr/create-reflect.test.tsx index 77c9087..5461596 100644 --- a/src/no-ssr/create-reflect.test.tsx +++ b/src/no-ssr/create-reflect.test.tsx @@ -229,3 +229,53 @@ describe('useUnitConfig', () => { ); }); }); + +describe('mapProps', () => { + const Greeting: FC<{ testId: string; label: string }> = (props) => { + return {props.label}; + }; + const greetingReflect = createReflect(Greeting); + + test('derives a prop in createReflect via mapProps', async () => { + const setName = createEvent(); + const $name = restore(setName, 'Bob'); + + const Hello = greetingReflect( + {}, + { + mapProps: { + label: { + source: $name, + fn: (name, props: { greeting: string }) => `${props.greeting}, ${name}!`, + }, + }, + }, + ); + + const container = render(); + expect(container.getByTestId('hello').textContent).toBe('Hi, Bob!'); + + await act(async () => { + setName('Alice'); + }); + expect(container.getByTestId('hello').textContent).toBe('Hi, Alice!'); + }); + + test('explicitly passed prop wins over the derived one and fn is skipped', () => { + const $name = createStore('Bob'); + const fn = vi.fn((name: string) => `Hello, ${name}!`); + + const Hello = greetingReflect( + {}, + { + mapProps: { + label: { source: $name, fn }, + }, + }, + ); + + const container = render(); + expect(container.getByTestId('hello').textContent).toBe('overridden'); + expect(fn).not.toHaveBeenCalled(); + }); +}); diff --git a/src/no-ssr/list.test.tsx b/src/no-ssr/list.test.tsx index e6170ac..e2cfade 100644 --- a/src/no-ssr/list.test.tsx +++ b/src/no-ssr/list.test.tsx @@ -1,6 +1,13 @@ import { list } from '@effector/reflect'; import { render } from '@testing-library/react'; -import { allSettled, createEffect, createEvent, createStore, fork } from 'effector'; +import { + allSettled, + createEffect, + createEvent, + createStore, + fork, + restore, +} from 'effector'; import { Provider, useStore } from 'effector-react'; import React, { FC, memo } from 'react'; import { act } from 'react-dom/test-utils'; @@ -565,3 +572,44 @@ describe('useUnitConfig', () => { ); }); }); + +describe('mapProps', () => { + const Item: FC<{ testId: string; label?: string }> = (props) => { + return
  • {props.label}
  • ; + }; + + test('derives a prop in list via mapProps using mapItem output', async () => { + const setName = createEvent(); + const $name = restore(setName, 'Bob'); + const $items = createStore([{ key: 'a' }, { key: 'b' }]); + + const Items = list({ + source: $items, + view: Item, + bind: {}, + mapItem: { + testId: (item) => item.key, + }, + mapProps: { + label: { + source: $name, + fn: (name, props: { testId: string }) => `${props.testId}-${name}`, + }, + }, + }); + + const container = render( +
      + +
    , + ); + expect(container.getByTestId('a').textContent).toBe('a-Bob'); + expect(container.getByTestId('b').textContent).toBe('b-Bob'); + + await act(async () => { + setName('Alice'); + }); + expect(container.getByTestId('a').textContent).toBe('a-Alice'); + expect(container.getByTestId('b').textContent).toBe('b-Alice'); + }); +}); diff --git a/src/no-ssr/reflect.test.tsx b/src/no-ssr/reflect.test.tsx index 29bce73..6c07c82 100644 --- a/src/no-ssr/reflect.test.tsx +++ b/src/no-ssr/reflect.test.tsx @@ -630,22 +630,42 @@ describe('mapProps', () => { expect(container.getByTestId('hello').textContent).toBe('Hi, Alice!'); }); - test('explicitly passed prop wins over the derived one', () => { + test('explicitly passed prop wins over the derived one and fn is skipped', () => { const $name = createStore('Bob'); + const fn = vi.fn((name: string) => `Hello, ${name}!`); const Hello = reflect({ view: Greeting, bind: {}, mapProps: { - label: { - source: $name, - fn: (name) => `Hello, ${name}!`, - }, + label: { source: $name, fn }, }, }); const container = render(); expect(container.getByTestId('hello').textContent).toBe('overridden'); + expect(fn).not.toHaveBeenCalled(); + }); + + test('bound store wins over mapProps and fn is skipped', () => { + const $a = createStore('from-bind'); + const $b = createStore('from-mapProps'); + const fn = vi.fn((b: string) => b); + + // The type-level fix (B1) makes this a type error — a key can't be in + // both bind and mapProps. We bypass it here to test the runtime skip, + // which is a defense-in-depth for JS users and type-bypass scenarios. + const Hello = reflect({ + view: Greeting, + bind: { label: $a }, + mapProps: { + label: { source: $b, fn }, + }, + } as any); + + const container = render(); + expect(container.getByTestId('hello').textContent).toBe('from-bind'); + expect(fn).not.toHaveBeenCalled(); }); test('combines an object of stores as source', async () => { diff --git a/src/no-ssr/variant.test.tsx b/src/no-ssr/variant.test.tsx index ecce031..7c6c4ab 100644 --- a/src/no-ssr/variant.test.tsx +++ b/src/no-ssr/variant.test.tsx @@ -2,7 +2,7 @@ import { variant } from '@effector/reflect'; import { act, render } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { createEvent, createStore, restore } from 'effector'; -import React from 'react'; +import React, { FC } from 'react'; test('matches first', async () => { const changeValue = createEvent(); @@ -323,3 +323,52 @@ describe('useUnitConfig', () => { ); }); }); + +describe('mapProps', () => { + const Greeting: FC<{ testId: string; label: string }> = (props) => { + return {props.label}; + }; + + test('derives a prop in variant via mapProps', async () => { + const setName = createEvent(); + const $name = restore(setName, 'Bob'); + const $type = createStore<'a' | 'b'>('a'); + + const Input = variant({ + source: $type, + bind: {}, + cases: { a: Greeting, b: Greeting }, + mapProps: { + label: { + source: $name, + fn: (name, props: { greeting: string }) => `${props.greeting}, ${name}!`, + }, + }, + }); + + const container = render(); + expect(container.getByTestId('hello').textContent).toBe('Hi, Bob!'); + + await act(async () => { + setName('Alice'); + }); + expect(container.getByTestId('hello').textContent).toBe('Hi, Alice!'); + }); + + test('explicitly passed prop wins over the derived one', () => { + const $name = createStore('Bob'); + const $type = createStore<'a' | 'b'>('a'); + + const Input = variant({ + source: $type, + bind: {}, + cases: { a: Greeting, b: Greeting }, + mapProps: { + label: { source: $name, fn: (name) => `Hello, ${name}!` }, + }, + }); + + const container = render(); + expect(container.getByTestId('hello').textContent).toBe('overridden'); + }); +}); diff --git a/src/ssr/create-reflect.test.tsx b/src/ssr/create-reflect.test.tsx index b76f7be..d02cf21 100644 --- a/src/ssr/create-reflect.test.tsx +++ b/src/ssr/create-reflect.test.tsx @@ -154,3 +154,39 @@ test('with ssr for client', async () => { const inputName = container.getByTestId('name') as HTMLInputElement; expect(inputName.value).toBe('Bob'); }); + +test('mapProps derives a prop in createReflect from a scoped store', async () => { + const app = createDomain(); + + const setName = app.createEvent(); + const $name = restore(setName, 'Bob'); + + const Greeting: FC<{ testId: string; label: string }> = (props) => { + return {props.label}; + }; + const greetingReflect = createReflect(Greeting); + + const Hello = greetingReflect( + {}, + { + mapProps: { + label: { + source: $name, + fn: (name, props: { greeting: string }) => `${props.greeting}, ${name}!`, + }, + }, + }, + ); + + const scope = fork(app, { values: [[$name, 'Alice']] }); + + const container = render( + + + , + ); + + expect(container.getByTestId('hello').textContent).toBe('Hi, Alice!'); + // global store is untouched + expect($name.getState()).toBe('Bob'); +}); diff --git a/src/ssr/list.test.tsx b/src/ssr/list.test.tsx index f06d6c8..c39950e 100644 --- a/src/ssr/list.test.tsx +++ b/src/ssr/list.test.tsx @@ -1,6 +1,6 @@ import { list } from '@effector/reflect/scope'; import { act, render } from '@testing-library/react'; -import { allSettled, createDomain, fork } from 'effector'; +import { allSettled, createDomain, fork, restore } from 'effector'; import { Provider, useStore } from 'effector-react/scope'; import React, { FC, memo } from 'react'; @@ -365,3 +365,43 @@ test('reflect-list: getKey option', async () => { fn2.mock.calls.map(([arg]) => arg), ); }); + +test('mapProps derives a prop in list from a scoped store', async () => { + const app = createDomain(); + + const setName = app.createEvent(); + const $name = restore(setName, 'Bob'); + const $items = app.createStore([{ key: 'a' }, { key: 'b' }]); + + const Item: FC<{ testId: string; label?: string }> = (props) => { + return
  • {props.label}
  • ; + }; + + const Items = list({ + source: $items, + view: Item, + bind: {}, + mapItem: { testId: (item: { key: string }) => item.key }, + mapProps: { + label: { + source: $name, + fn: (name, props: { testId: string }) => `${props.testId}-${name}`, + }, + }, + }); + + const scope = fork(app, { values: [[$name, 'Alice']] }); + + const container = render( + +
      + +
    +
    , + ); + + expect(container.getByTestId('a').textContent).toBe('a-Alice'); + expect(container.getByTestId('b').textContent).toBe('b-Alice'); + // global store is untouched + expect($name.getState()).toBe('Bob'); +}); diff --git a/src/ssr/variant.test.tsx b/src/ssr/variant.test.tsx index 48964dc..1c21ca8 100644 --- a/src/ssr/variant.test.tsx +++ b/src/ssr/variant.test.tsx @@ -1,9 +1,9 @@ import { variant } from '@effector/reflect/scope'; import { act, render } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { allSettled, createDomain, fork, restore } from 'effector'; +import { allSettled, createDomain, createStore, fork, restore } from 'effector'; import { Provider } from 'effector-react/scope'; -import React from 'react'; +import React, { FC } from 'react'; test('matches first', async () => { const app = createDomain(); @@ -239,3 +239,39 @@ function InputCustom3(props: { /> ); } + +test('mapProps derives a prop in variant from a scoped store', async () => { + const app = createDomain(); + + const setName = app.createEvent(); + const $name = restore(setName, 'Bob'); + const $type = app.createStore<'a' | 'b'>('a'); + + const Greeting: FC<{ testId: string; label: string }> = (props) => { + return {props.label}; + }; + + const Input = variant({ + source: $type, + bind: {}, + cases: { a: Greeting, b: Greeting }, + mapProps: { + label: { + source: $name, + fn: (name, props: { greeting: string }) => `${props.greeting}, ${name}!`, + }, + }, + }); + + const scope = fork(app, { values: [[$name, 'Alice']] }); + + const container = render( + + + , + ); + + expect(container.getByTestId('hello').textContent).toBe('Hi, Alice!'); + // global store is untouched + expect($name.getState()).toBe('Bob'); +}); diff --git a/type-tests/types-list.tsx b/type-tests/types-list.tsx index ff483ee..3c69508 100644 --- a/type-tests/types-list.tsx +++ b/type-tests/types-list.tsx @@ -196,3 +196,44 @@ import { expectType } from 'tsd'; expectType(List); } + +// --- review B2: a key in both `mapItem` and `mapProps` is a type error ------- +// Note: this is caught for unannotated arrow functions. If the user explicitly +// annotates the `item` parameter, TS's excess-property checking is bypassable +// (known TS limitation with mapped-type `extends` constraints). + +// B2 negative: same key in mapItem and mapProps — collision +{ + const Item: React.FC<{ id: string; label?: string }> = () => null; + const $name = createStore(''); + const $items = createStore<{ id: string }[]>([{ id: 'a' }]); + + list({ + source: $items, + view: Item, + bind: {}, + mapItem: { + id: (item) => item.id, + // @ts-expect-error - `label` is in mapProps; mapItem omits it + label: (item) => item.id, + }, + mapProps: { label: { source: $name, fn: (n) => n } }, + }); +} + +// B2 positive: disjoint keys in mapItem and mapProps — compiles +{ + const Item: React.FC<{ id: string; label?: string }> = () => null; + const $name = createStore(''); + const $items = createStore<{ id: string }[]>([{ id: 'a' }]); + + const List = list({ + source: $items, + view: Item, + bind: {}, + mapItem: { id: (item) => item.id }, + mapProps: { label: { source: $name, fn: (n) => n } }, + }); + + expectType(List); +} diff --git a/type-tests/types-reflect.tsx b/type-tests/types-reflect.tsx index f5415e4..c212b0c 100644 --- a/type-tests/types-reflect.tsx +++ b/type-tests/types-reflect.tsx @@ -1,7 +1,7 @@ /* eslint-disable @typescript-eslint/ban-ts-comment */ import { reflect } from '@effector/reflect'; import { Button } from '@mantine/core'; -import { createEvent, createStore } from 'effector'; +import { createEvent, createStore, Store } from 'effector'; import React, { AnchorHTMLAttributes, ButtonHTMLAttributes, @@ -656,7 +656,9 @@ function localize(value: string): unknown { expectType(App); } -// mapProps: a key that is not a prop of the view makes fn's return type `never` +// mapProps: a key that is not a prop of the view is a type error on the key +// (under the stricter `Record` constraint). +// The `@ts-expect-error` sits on the key line, not on `fn`'s body. { const Greeting: React.FC<{ label: string; @@ -667,10 +669,10 @@ function localize(value: string): unknown { view: Greeting, bind: {}, mapProps: { + // @ts-expect-error - `unknownProp` is not a prop of `Greeting` unknownProp: { source: $name, - // @ts-expect-error - `unknownProp` is not a prop, so the return type is `never` - fn: (value) => value, + fn: (value: any) => value, }, }, }); @@ -747,3 +749,96 @@ function localize(value: string): unknown { const App: React.FC = () => ; expectType(App); } + +// --- review #2: typo'd key errors on the key itself, not on fn's return -- +{ + const Greeting: React.FC<{ label: string }> = () => null; + const $name = createStore(''); + + reflect({ + view: Greeting, + bind: {}, + mapProps: { + // @ts-expect-error - `labe` is not a prop of `Greeting`; error on the key + labe: { source: $name, fn: (name) => name }, + }, + }); +} + +// --- review #3: `source` passed as a variable ---------------------------- + +// 3a: object source assigned to a const - `value` stays precisely typed +{ + const Greeting: React.FC<{ label: string; currency: string }> = () => null; + const $cart = createStore<{ count: number }>({ count: 0 }); + const $name = createStore(''); + + const src = { cart: $cart, name: $name }; + + const Reflected = reflect({ + view: Greeting, + bind: {}, + mapProps: { + label: { + source: src, + fn: (s, props) => `${s.name}: ${s.cart.count} ${props.currency}`, + }, + }, + }); + + const App: React.FC = () => ; + expectType(App); +} + +// 3b: source typed as `Store` - `value` degrades to `any` (accepted TS limit) +{ + const Greeting: React.FC<{ label: string }> = () => null; + const $name = createStore(''); + const loose: Store = $name; + + reflect({ + view: Greeting, + bind: {}, + mapProps: { + label: { + source: loose, + // `v` is `any` - no error, but no safety either + fn: (v) => v.unknownField.thatsNotChecked, + }, + }, + }); +} + +// --- review B1: a key in both `bind` and `mapProps` is a type error ------- + +// B1 negative: same key in bind and mapProps — collision +{ + const V: React.FC<{ a: string; b: string }> = () => null; + const $a = createStore('a'); + const $b = createStore('b'); + + reflect({ + view: V, + bind: { a: $a }, + mapProps: { + // @ts-expect-error - `a` is in bind; collision with mapProps + a: { source: $b, fn: (b) => b }, + }, + }); +} + +// B1 positive: disjoint keys in bind and mapProps — compiles +{ + const V: React.FC<{ a: string; b: string }> = () => null; + const $a = createStore('a'); + const $b = createStore('b'); + + const R = reflect({ + view: V, + bind: { a: $a }, + mapProps: { b: { source: $b, fn: (b) => b } }, + }); + + const App: React.FC = () => ; + expectType(App); +}