Flow: Element props marked as mixed since 0.89.0

[villesau]

try flow

Following code:

/* @flow */
import * as React from 'react';
type Props = {a: number}
type Com = React.Element<React.ComponentType<Props>>;

const fn = (com: Com) => {
    (com.props: Props);
}

errors since com.props is mixed. it works if using React.Element>, or React.Element>> but obviously that is often too strict requirement. looks like React.StatelessFunctionalComponent | Class> would work too which is odd since by the documentation, it should be pretty much the same as React.ComponentType. It's just crazy cumbersome way to say that "Hey, I want an element which has these props"

As seen from this code sandbox, accessing props from component instance is fine.

This works in 0.88.0 and earlier versions, but in 0.89.0 props are marked as mixed.

Btw, would be good if there would be something like React.ElementType<Props> that would do the same as React.Element<React.ComponentType<Props>> is supposed to do.

This issue makes it very difficult for us to update to latest Flow.

[jbrown215]

React.ComponentType is no longer the union of class and function components. Now, it is an alias for AbstractComponent, which gives mixed for ElementProps.

The reason for this is that the ElementProps type destructor only really used to work on classes or functions, but nothing wrapped in an HOC. We chose not to support it with AbstractComponent because we would need to also propagate DefaultProps as a type argument to use it.

If you want to use ElementProps in a way that Flow can give you useful information, you can define a type alias to be the union of function and class components:

type FunctionOrClassComponent<Props> = React$StatelessFunctionalComponent<Props> | Class<React$Component<Props>>

and use that type where you need ElementProps instead of ComponentType.

[villesau]

Ok, thanks! How open are you to introduce that kind of utility types in React.SomeUtilityType form? React.ElementType<Props> and React.FunctionOrClassComponent<Props> would still be super useful, at least for our usage patterns. Or is it just recommended to build some own utility libraries instead?

[jbrown215]

For now, I recommend just adding those aliases in your own libdef. If more people start to request this, then I will re-evaluate.

[jbrown215]

@villesau: Does ElementProps returning the Config on AbstractComponent work for your use case?

[villesau]

@jbrown215 I'm not exactly sure what you mean? Where to put ElementProps? I need to explicitly access props from com.props in this case, not just the type. The example is simplified, but there are cases where some a bit smarter components needs to access element properties and interact with them.

[jbrown215]

Right, I meant to ask if the type of com.props being typed as the Config of com (instead of the props of com) would be sufficient. We're discussing more precision for ElementProps with AbstractComponent internally. I'll keep you posted on what we decide, which may be helpful here.

[namuol]

React.ComponentType is no longer the union of class and function components. Now, it is an alias for AbstractComponent, which gives mixed for ElementProps.

@jbrown215 Why was this breaking change made? I assume there's some good technical reason, but from a standard library standpoint this is very disruptive.

Our codebase, and likely a lot of library definitions still operate under the assumption that ComponentType refers to Stateless | Class components.

For now, I recommend just adding those aliases in your own libdef.

This should help us move forward; thank you for clarifying :+1:

If more people start to request this, then I will re-evaluate.

I do think a general purpose ComponentType is very useful. Most React code doesn't "care" about whether a component is stateless or stateful, so most abstractions that work with components will probably need to rely on their own duplicate alias, which will lead to more confusing error messages in the long run.

[jbrown215]

Why was this breaking change made? I assume there's some good technical reason, but from a standard library standpoint this is very disruptive.

ComponentType as a union of function components and class components wasn't sufficient to model react components that were released in the last year. A few of these components include React.StrictMode, React.ConcurrentMode, React.Fragment, React.memo, React.lazy, React.forwardRef, etc.

In order to provide a more maintainable abstraction for React, we created AbstractComponent. Class Components and Function components are both AbstractComponents, but all the newer react components are also AbstractComponents. AbstractComponent keeps track of the Config (Props with all DefaultProps marked optional) and Instance of a component, which we hope will be enough to model more react components that are as of yet unreleased.

ElementProps was the only use case that required that we still propagate information about the default props separately from the config in AbstractComponent. That use case did not seem like it was worth propagating a third type parameter for in AbstractComponent, which we tried to keep as ergonomic as possible.

I do think a general purpose ComponentType is very useful

AbstractComponent _is_ that general purpose type for referring to all components. It precisely does not care about the state (or lack thereof) in a component-- all it cares about is the config type, which is at an even higher level of abstraction than props and default props.

When I said I'd re-evaluate, I was talking specifically about the decision to _not_ propagate DefaultProps in AbstractComponent.

If you want to read more about AbstractComponent, check out our blog post: https://medium.com/flow-type/supporting-react-forwardref-and-beyond-f8dd88f35544

[namuol]

Thanks for the explanation @jbrown215 🙇

I'm writing an upgrade guide for my team right now; is it safe to say ElementConfig is _probably_ what you want to use in most cases where you would have used ElementProps, previously?

For example:

import * as React from 'react';

declare var Foo: React.ComponentType<{|foo: number|}>;

type FooProps = React.ElementProps<typeof Foo>;
type FooConfig = React.ElementConfig<typeof Foo>;

declare var FromElementProps: React.ComponentType<FooProps>;
declare var FromElementConfig: React.ComponentType<FooConfig>;

<FromElementProps />; // `foo` missing, but no error detected 🙈

<FromElementConfig />; // `foo missing, error detected 👍

(Try on flow.org/try)

[villesau]

@namuol can you make the guide public? :) I'm sure it would be very helpful for the community!

[jbrown215]

@namuol: Mind showing a less contrived example? There's no need to use ElementProps or ElementConfig there since using Foo directly has the behavior you want.

[namuol]

can you make the guide public?

@villesau Maybe later after I'm more confident about the general advice I'd give - right now the guide is tailored for our specific codebase

There's no need to use ElementProps or ElementConfig there since using Foo directly has the behavior you want.

@jbrown215 One of the most frequent usages of ElementProps was to provide a concrete P type for HOCs like React.memo:

React.memo<React.ElementProps<typeof Foo>>(Foo);

// or

(React.memo(Foo): React.ComponentType<React.ElementProps<typeof Foo>>);

But we also use it in other places to share/extend prop types across components:

type BarProps = {
  ...React.ElementProps<typeof Foo>,
  bar: number,
};

Here's an actual example from our codebase:

type PopoverBalloonProps = {
  ...React.ElementProps<typeof Balloon>,
  anchor: ?Box,
  rect: ?Box,
  position: PositionIdentifier,
  children: React.Node,
};

In the above example, post-0.89, the concrete type for PopoverBalloonProps becomes:

type PopoverBalloonProps = {
  [string]: mixed,
  anchor: ?Box,
  children: Node,
  position: PositionIdentifier,
  rect: ?Box
};

Switching to ElementConfig fixes the issue in this case and many other similar usages :+1:

[jbrown215]

Mind showing how you specify the Props for Balloon? I recommend _not_ using the type destructors if you can avoid it. AbstractComponent gets rid of the need to use ElementConfig.

If I were writing Balloon and wanted to extend its Props for another component somewhere, here's how I would do it:

Balloon.js

export type Props = {|
  // Balloon's props
|};

function BalloonComponent(props: Props): React.Node {
 // ...
}

module.exports = BalloonComponent;

PopoverBalloon.js

import type {Props} from './Balloon';

type PopoverBalloonProps = {|
  ...Props,
  // Extra props
|}

Mind joining the discord or opening a new issue to discuss further? I'm happy to help you get a recommended usage guide down.

https://discord.gg/8ezwRUK

[edahlseng]

@jbrown215 was there any helpful information discussed in discord about this issue that could be summarized here? I'm running into a similar issue, but am unable to fix it in the way that you describe above, as it's a HOC (so we are inferring Props instead of being able to import it).

[jbrown215]

@edahlseng: Mind sharing a bit more about your use case? I don't think the discussion @namuol and I had will generalize well to everyone, but here is a summary of how I recommend to use ElementConfig after 0.89. Tl;dr, you probably shouldn't use it anymore:

The main purpose for ElementConfig prior to 0.89.0 was for writing HOCs and preserving the "optionality" of defaultProps on the resulting component. Since AbstractComponent already changes Prop+DefaultProps into a config, using ElementConfig is no longer necessary in HOCs.

Using ElementConfig will not give you the same Props that you parameterize a component with if defaultProps are specified. ElementConfig gets the props of a component and marks all defaultProps optional, so this won't always work as you expect it to. I really do recommend exporting the Props types directly, but if you can't then ElementConfig is probably the next best bet.

I recommend 1 exported component per module with a toplevel exported "Props" type alias so that it's easy to import a Component and its Props type.

[edahlseng]

@jbrown215 thanks, it's helpful to know some more of the background on this!

The real-world use case that I'm running into issues with is with updating the flow-typed defs for styled-components (flow-typed PR #3165).

I tried putting together a minimal reproducing case, and am even getting different errors than what libdefs are encountering, but may still illustrate the root issue:

import React from 'react';

type Props = {
  name: string,
  count: number,
  background: string
}

const TestComponent = (props: Props) => null

type Interpolation<P: {}> = (executionContext: P) => string;

const styled = <Config: {}>(Component: React$AbstractComponent<Config, *>) => (strings: string[], ...interpolations: Array<Interpolation<Config>>): React$AbstractComponent<Config, *> => Component;

// This fails, saying that `background` does not exist in `props`
const StyledTestComponent = styled(TestComponent)`
    color: ${props => props.background}
`;

[jbrown215]

I'm unfamiliar with the backtick syntax you're using. Mind explaining?

Also, instead of * you can use mixed

[villesau]

@jbrown215 I believe it's styled components using tagged template literals ( https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals )

[jbrown215]

Ah, got it. Fancy stuff there, JS.

@edahlseng: Flow doesn't infer generics, which is what I think is causing the error here. It's hard for me to parse styled, but I think you need to annotate the return type of the outer function. Here it is rewritten a bit and working as expected.

[edahlseng]

Ahh, thanks @jbrown215, that rewrite was helpful. I didn't realize that was a limitation.

Is this a fundamental limitation? It would be awesome if some day Flow could just infer these, and I'm curious what it would take to get there.

[jbrown215]

@edahlseng It is a fundamental limitation of the inference algorithm we use :(

[goodmind]

Is this resolved? @villesau

[jbrown215]

@goodmind: I'm open to considering improvements again in the future, but for now this is safe to close.