Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix final props type inference #87

Merged
merged 6 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading