Skip to content

Commit

Permalink
Fix final props type inference (#87)
Browse files Browse the repository at this point in the history
* Add type test for current behavoir (keys from bind excluded from final props)

* Update test to match desired behavior (keys from bind are optional in final props)

* Update variant type tests to match desired behavior

* Update types to match desired behavior

* Add more type tests

* Use Show from effector to enforce better types in IDE
  • Loading branch information
AlexandrHoroshih authored Feb 9, 2024
1 parent 60f5c02 commit 4c8b159
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 5 deletions.
26 changes: 22 additions & 4 deletions public-types/reflect.d.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable @typescript-eslint/consistent-type-definitions */
import type { EventCallable, Store } from 'effector';
import type { EventCallable, Show, Store } from 'effector';
import type { useUnit } from 'effector-react';
import type { ComponentType, FC, PropsWithChildren, ReactHTML } from 'react';

Expand All @@ -12,6 +12,12 @@ type Hooks = {
unmounted?: EventCallable<void> | (() => unknown);
};

/**
* `bind` object type:
* prop key -> store (unwrapped to reactive subscription) or any other value (used as is)
*
* Also handles some edge-cases like enforcing type inference for inlined callbacks
*/
type BindFromProps<Props> = {
[K in keyof Props]?: K extends UnbindableProps
? never
Expand All @@ -25,6 +31,18 @@ type BindFromProps<Props> = {
: Store<Props[K]> | Props[K];
};

/**
* Computes final props type based on Props of the view component and Bind object.
*
* Props that are "taken" by Bind object are made **optional** in the final type,
* so it is possible to owerrite them in the component usage anyway
*/
type FinalProps<Props, Bind extends BindFromProps<Props>> = Show<
Omit<Props, keyof Bind> & {
[K in Extract<keyof Bind, keyof Props>]?: Props[K];
}
>;

// relfect types
/**
* Operator that creates a component, which props are reactively bound to a store or statically - to any other value.
Expand All @@ -49,7 +67,7 @@ export function reflect<Props, Bind extends BindFromProps<Props>>(config: {
* This configuration is passed directly to `useUnit`'s hook second argument.
*/
useUnitConfig?: UseUnitConfig;
}): FC<Omit<Props, keyof Bind>>;
}): FC<FinalProps<Props, Bind>>;

// Note: FC is used as a return type, because tests on a real Next.js project showed,
// that if theoretically better option like (props: ...) => React.ReactNode is used,
Expand Down Expand Up @@ -83,7 +101,7 @@ export function createReflect<Props, Bind extends BindFromProps<Props>>(
*/
useUnitConfig?: UseUnitConfig;
},
) => FC<Omit<Props, keyof Bind>>;
) => FC<FinalProps<Props, Bind>>;

// list types
type PropsifyBind<Bind> = {
Expand Down Expand Up @@ -199,7 +217,7 @@ export function variant<
*/
useUnitConfig?: UseUnitConfig;
},
): FC<Omit<Props, keyof Bind>>;
): FC<FinalProps<Props, Bind>>;

// fromTag types
type GetProps<HtmlTag extends keyof ReactHTML> = Exclude<
Expand Down
88 changes: 88 additions & 0 deletions type-tests/types-reflect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,35 @@ import { expectType } from 'tsd';
expectType<React.FC>(ReflectedInput);
}

// reflect should not allow wrong props in final types
{
const Input: React.FC<{
value: string;
onChange: (newValue: string) => void;
color: 'red';
}> = () => null;
const $value = createStore<string>('');
const changed = createEvent<string>();

const ReflectedInput = reflect({
view: Input,
bind: {
value: $value,
onChange: changed,
},
});

const App: React.FC = () => {
return (
<ReflectedInput
// @ts-expect-error
color="blue"
/>
);
};
expectType<React.FC>(App);
}

// reflect should allow not-to pass required props - as they can be added later in react
{
const Input: React.FC<{
Expand Down Expand Up @@ -104,6 +133,65 @@ import { expectType } from 'tsd';
expectType<React.FC>(AppFixed);
}

// reflect should make "binded" props optional - so it is allowed to overwrite them in react anyway
{
const Input: React.FC<{
value: string;
onChange: (newValue: string) => void;
color: 'red';
}> = () => null;
const $value = createStore<string>('');
const changed = createEvent<string>();

const ReflectedInput = reflect({
view: Input,
bind: {
value: $value,
onChange: changed,
},
});

const App: React.FC = () => {
return <ReflectedInput value="kek" color="red" />;
};

const AppFixed: React.FC = () => {
return <ReflectedInput color="red" />;
};
expectType<React.FC>(App);
expectType<React.FC>(AppFixed);
}

// reflect should not allow to override "binded" props with wrong types
{
const Input: React.FC<{
value: string;
onChange: (newValue: string) => void;
color: 'red';
}> = () => null;
const $value = createStore<string>('');
const changed = createEvent<string>();

const ReflectedInput = reflect({
view: Input,
bind: {
value: $value,
onChange: changed,
color: 'red',
},
});

const App: React.FC = () => {
return (
<ReflectedInput
// @ts-expect-error
color="blue"
/>
);
};
expectType<React.FC>(App);
}

// reflect should allow to pass EventCallable<void> as click event handler
{
const Button: React.FC<{
Expand Down
1 change: 0 additions & 1 deletion type-tests/types-variant.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,6 @@ import { expectType } from 'tsd';
<ComponentWithVariantAndReflect slot={<h2>Another slot type(</h2>} />
<ComponentWithVariantAndReflect
slot={<h2>Should report error for "name"</h2>}
// @ts-expect-error
name="kek"
/>
</main>
Expand Down

0 comments on commit 4c8b159

Please sign in to comment.