-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
HOC returned component props can not differ from HOC generic props #28938
Comments
I believe I'm having the exact same problem on TypeScript 3.2.2, and I'd like to share my use case to show another way this is affecting HOCs in React. In my case I have a HOC that grabs Here's a simplified (React Native) code: import * as React from 'react';
import { ViewProps } from 'react-native';
export const withXBehavior = <P extends ViewProps>(
WrappedComponent: React.ComponentType<P>,
): React.ComponentType<P> => (props) => {
const { children, ...otherProps } = props;
return (
<WrappedComponent {...otherProps}> /* ERROR HERE */
<React.Fragment>
<View>
<Text>example of HOC stuff here</Text>
</View>
{children}
</React.Fragment>
</WrappedComponent>
);
}; Error: This was working fine on 3.0.1 and is now giving me this error on 3.2.2 (haven't tested on 3.1). In my real world case I need
If I forward all HOC props to WrappedComponent the error is gone, but that would prevent HOCs to change behavior of WrappedComponents. import * as React from 'react';
import { Text, View, ViewProps } from 'react-native';
export const withXBehavior = <P extends ViewProps>(
WrappedComponent: React.ComponentType<P>,
): React.ComponentType<P> => (props) => {
const { children } = props;
return (
<WrappedComponent {...props}>
<React.Fragment>
<View>
<Text>example of HOC stuff here</Text>
</View>
{children}
</React.Fragment>
</WrappedComponent>
);
}; No errors in the above code. |
I'm seeing the same thing as the original poster, but from version 3.1.6 to 3.2.2 |
@ahejlsberg The change is that we no longer erase generics in JSX (so we actually check these calls now), and roughly that |
this turned out to be really messy. first of all, typing HOCs is not really easy. but once I started to get going with it, I hit a TypeScript 3.2 bug that prevents the HOC typing trick from working. This would've worked in earlier TSC versions. See: microsoft/TypeScript#28938
Just for the sake of completeness: In some cases we do not remove the prop from the returned component, but make it optional instead. This also fails for the same reason: import * as React from 'react';
export interface HOCProps {
foo: number;
}
/** From T make every property K optional */
type Partialize<T, K extends keyof T> = Pick<T, Exclude<keyof T, K>> & Partial<Pick<T, K>>;
/** Remove props, that have been prefilled by the HOC */
type OptionalPrefilled<T extends HOCProps> = Partialize<T, keyof HOCProps>;
export default function<P extends HOCProps>(WrappedComponent: React.ComponentType<P>) {
return class SomeHOC extends React.Component<OptionalPrefilled<P>> {
public render(): JSX.Element {
return <WrappedComponent foo={0} {...this.props} />;
}
};
} Error:
So basically it’s the same. I just wanted to provide another example where TypeScript does not recognize that the type is assignable to T. This could be harder to solve then the original one though. |
It looks like the same problem as in #28884. It fails even if no spread rest. |
Starting with 3.2 the behaviour of the spread operator for generics has changed. Apparently the type of |
Is there a bug somewhere where we can track this specific problem? |
@weswigham Is this the same as #28748? |
Had to use |
* Rename all js, jsx to ts, tsx * Add typescript configuration * Add naive flow -> typescript convertor * Add configuration to fix typescript code * Convert flow to typescript * Add more typings * Fix double React bug * Fix some simple typings * Fix ExternalLink typing * Fix ScrollToTop and React util * Fix CORS notification * Fix TimeslotTable * Fix SearchBox * Fix config * Fix venues * Fix CORS * Fix StaticPage * Fix query-string type version * Fix ModulePageContainer test * Fix some more stuff * Fix custom module form * Fix AddModule * Fix TodayContainer * Fix RefreshPrompt * Fix VenueContainer * Fixing things * Revert e2e test code to JS * Update packages * Fix stuff * Delete all Flow typedefs * Fix even more stuff * Update some configs * Fix up eslint * Fixing stuff * Fixing more code * Update stuff * Maybe fix CI * Fix stuff * Fix more stuff * Add simple fixes * Add more simple fixes * Fix footer * Fix image types and global window extension * Fix all reducers except undoHistory * Fix global search test * Add NUSModerator libdef * Fix ExamCalendar * Fix up Redux actions * Fix bootstrapping (except config store * Fix up test-utils * Fix timetable export typing * Fix today views * Update planner types * Fix layout components * Fix simple components * Update global and installed typings Downgrade Jest typings to avoid bug with mockResolve/Reject * Update module-info and ButtonGroup component * Fix weekText test * Fix typing for module pages Still need to add a libdef for react-scrollspy * Fix component types TODO: Figure out how to type dynamic imports * Fix selectors * Fix planner reducer * Fix Contribute container * Improve venue typing TODO: Fix react-leaflet types, timetable types * Fix error components * Fix middleware typing * Fix settings views * Fix routes typing * Fix venueLocation JSON typing * Fix CorsNotification test * Fix storage strict errors * Improve util types * Add React Kawaii libdef * Fix react-feather import * Improve timetable typing * Skip checking .d.ts files * Fix more types * Fix filter views * Mark Disqus page as any * Add json2mq types and fix CSs util * Fix share timetable * Fix BusStops and split ArrivalTimes * Add react-scrollspy libdef * Fix colors util * Fix module page views * Add react leaflet types * Fix ImproveVenueForm * Incomplete fixes for planner and filters * Fix modulePage route * Hack board.ts * Fix request middleware test typing * Fix Sentry typing * Add more leaflet typings * Fix TetrisGame * Fix reducers test * Add no-scroll libdef * Fix BusStops test * Fix api types * Add typing for weather API responses * Fix moduleBank and requests tests * Fix minor bugs in planner and modtris typing * Fix history debouncer test mock typing * Fix storage types * Ignore some minor TS errors * Realign Timetable types * Improve HOC typing * Fix iCal tests * Improve timetable test types * Fix up more types * Run eslint autofix * Fix undoHistory type * Fix lint errors * Fix SVG component typing * Fix filter groups * Fix test configs * Update snapshots * Fix hoc typing See: microsoft/TypeScript#28938 (comment) * Fix jest tests * Remove flow typings * Fix timetable-export webpack config * Clean up tsconfig * Improve babel config * Fix lint errors and improve typing * Remove and simplify lint ignores * Clean up more types * Directly import react-feather icons * Remove migration script * Remove final traces of flow * Clean up Babel configs * Minor fixes and comments * Remove unused packages * Update README and add tsc to CI
I think that this issue can be closed, since the implicit types of the spread operator were changed. This is a fully working example you can use with TS 3.5+ (or for 3.2+ use
Of course, if you need to shadow only one property explicitly, you don't need the |
I'm not sure if this is specific to my environment @lvkins , but I was getting 'keyof T2 does not satisfy the constraint keyof T1' which dropped to 'type string is not assignable to type keyof T1'. Fixed by specifying T1 as an extension of T2:
Let me know if this is incorrect, am on 3.8.2, been using typescript for a while but sometimes I've troubles grokking the complexities of typing. Thanks! |
@lvkins the idea here is to force my case exampleimport React, { ComponentType } from 'react'
import { CountrySettingsFlags } from '../country-settings-flags.enum'
import { useCountries } from './countries.hook'
type WithCountriesProps = {
countries: ReturnType<typeof useCountries>
}
function withCountries(flags: CountrySettingsFlags[] = []) {
return function <Props extends WithCountriesProps>(Component: ComponentType<Props>) {
return function (props: Omit<Props, keyof WithCountriesProps>) {
const countries = useCountries(flags)
return <Component {...props as Props} countries={countries} />
// ^^^^^^^^
// without this will also error, what is describing this issue
}
}
}
type TestProps = {
the: number
one: string
}
function Test(props: TestProps) {
return null
}
const Wrapped = withCountries()(Test)
// ^^^^
// error because `TestProps` does not extend `WithCountriesProps`
const test = <Wrapped the={666} one="" /> |
I'm kind of reviving this thread but casting is not a proper solution, it's highly unsafe. Here's a sample:
This sample is totally valid even |
I've tried to do this to avoid type assertion: type InjectedProps = {
foo: string
}
type PropsWithoutInjected<TBaseProps> = Omit<
TBaseProps,
keyof InjectedProps
>
export function withFoo<TProps extends InjectedProps>(
WrappedComponent: React.ComponentType<
PropsWithoutInjected<TProps> & InjectedProps
>
) {
return (props: PropsWithoutInjected<TProps>) => {
return <WrappedComponent {...props} foo="bar" />
}
} This way the wrapped component can't receive the injected props and TypeScript does not complain. Here is a TypeScript playground with an example of how this HOC can be used. |
@satansdeer but you're not getting the rest props, it should be |
@gabriellsh not sure why do you need to get the |
Ok here is another take from @dex157: export interface WithFooProps {
foo: number;
}
export function withFoo<ComponentProps>(WrappedComponent: React.ComponentType<ComponentProps & WithFooProps>) {
return class FooHOC extends React.Component<ComponentProps> {
public render() {
return <WrappedComponent {...this.props} foo={0} />;
}
};
} This way we don't even have to use interface ButtonProps {
title: string;
}
const Button: React.ComponentType<ButtonProps> = withFoo(({title, foo}) => {
return <button id={String(foo)}>{title}</button>
})
const App = () => {
return <main><Button title="click me" /></main>
} Here is a TypeScript playground to try it out. |
Basically the difference in @satansdeer answer is to use type (which is not stated there explicitly) instead of interfaces This will not work import * as React from 'react';
export interface WithFooProps {
foo: number;
}
interface ButtonProps extends WithFooProps {
title: string;
}
export function withFoo<ComponentProps>(WrappedComponent: React.ComponentType<ComponentProps & WithFooProps>) {
return (props: ComponentProps) => {
return <WrappedComponent {...props} foo={0} />;
}
};
const RealButton: React.FC<ButtonProps> = ({title, foo}) => {
return <button id={String(foo)}>{title}</button>
}
const Button = withFoo(RealButton)
const App = () => {
return <main><Button title="click me" /></main>
} And this will import * as React from 'react';
export interface WithFooProps {
foo: number;
}
type ButtonProps= {
title: string;
} & WithFooProps
export function withFoo<ComponentProps>(WrappedComponent: React.ComponentType<ComponentProps & WithFooProps>) {
return (props: ComponentProps) => {
return <WrappedComponent {...props} foo={0} />;
}
};
const RealButton: React.FC<ButtonProps> = ({title, foo}) => {
return <button id={String(foo)}>{title}</button>
}
const Button = withFoo(RealButton)
const App = () => {
return <main><Button title="click me" /></main>
} The difference is in ButtonProps |
@coalaroot great catch. Yes, using the |
will... how about generics in FooProps? :)) export interface WithFooProps<T> {
foo: T;
} |
Can do but why though? |
Sorry man, don't have the time to figure it out :( |
This is the simplest reproducible sample code type Foo = {
foo: string;
};
function addFoo<P extends Foo>() {
return (argWithoutFoo: Omit<P, keyof Foo>) => {
const argWithFoo: P = { ...argWithoutFoo, foo: "foo" };
};
} This raises the following error:
|
@hayata-suenaga weird thing is, this works: type Foo = {
foo: string;
};
function addFoo<P extends Foo>() {
return (argWithoutFoo: Omit<P, keyof Foo>) => {
const argWithFoo: Omit<P, keyof Foo> & Pick<P, keyof Foo> = { ...argWithoutFoo, foo: "foo" };
};
} |
TypeScript Version: 3.3.0-dev.20181208
Search Terms:
Code
Expected behavior:
No error, like with every version below 3.2.0.
Actual behavior:
Throws an error highlighting the WrappedComponent in the render method.
Additional Information
This is pretty much the same sample example used in #28720, but with the difference, that the props of the returned component differ from the generic.
Basically the HOC prefills the
foo
property for theWrappedComponent
. Since the spreaded props are overriden byfoo
, I don’t wantfoo
to be a valid property for the HOC. This does not seem to be possible anymore.Playground Link:
Related Issues:
#28720
The text was updated successfully, but these errors were encountered: