GeezerVison

Would it be worth considering adding type Omit<T, K extends keyof T> = Pick<T, Exclude<keyof T, K>> as a predefined type and using that here?

Would it be worth considering adding type Omit<T, K extends keyof T> = Pick<T, Exclude<keyof T, K>> as a predefined type and using that here?

We considered that, but there are too many projects out there that already define their own Omit and we'd end up with duplicate definition issues.

Is this scenario is not supported? Or this is a bug?

function foo<T>(obj: T & { x: string }): T {
    const { x, ...rest } = obj;
    return rest; // Error: [ts] Type 'Pick<T & { x: string; }, Exclude<keyof T, "x">>' is not assignable to type 'T'. [2322]
}

Veniamin Krol (@vkrol) if T contains a property x it will not be present after the destructuring. Therefore the error is correct.

Klaus Meinhardt (@ajafff) But T does not contains x, T & { x: string } contains x 🤔.

Veniamin Krol (@vkrol) Let T={ x: number }. Now T contains x.

Klaus Meinhardt (@ajafff) While this is correct, I think it’s actually undesired. Most of the motivation for the higher-order definition of spread as intersection was that it’s quite usable, as well as correct [1] for the common case where two disjoint types are spread together (the case here) or the same type is spread into itself.

The definition of rest we ended up with is actually more correct, as you note, but it means that it doesn’t have the same assignability behaviour as spread. I think the correct fix here would be to add a simplification rule in assignability checking that says that Pick<T & U, Exclude<keyof T, keyof U>> is assignable to T.

However, I’m not sure if such a rule would be applicable anywhere else, and I think it would be fiddly to get right since U and keyof U are not tightly linked, so you’d have to relate them via assignability instead of just checking type equality.

[1] except of course for own-ness, which the compiler doesn’t track well anyway.

What about following scenario Nathan Shively-Sanders (@sandersn) ( related to react HoC, but the same behaviour happens with raw functions )

Error:

Fix:

It needs to be casted to original P to get rid of errors, although InjectedProps are subtracted from P constraint within the implementation and the exact shape is properly passed via const injectedProps

So is this correct behaviour or a bug ?

Martin Hochel (@Hotell)

As far as I understand the problem, I think it's the same as Veniamin Krol (@vkrol)'s. I'm not sure I understand it, however.

If InjectedProps is the type that's related to withCounter, why does counterWannabe have to declare it as a type at all? Shouldn't extendedFunc be the one with the dependency on the counter-related types InjectedProps and ExtendedProps?

In other words, why does P extend InjectedProps? Shouldn't it be unconstrained and then innerFunc be (props: P & ExtendedProps) => P & InjectedProps ?

Of course, given the const { maxCount, ...passThroughProps } = props and the problem Veniamin Krol (@vkrol) lays out above, you'll get Pick<P & ExtendedProps, Exclude<keyof P & ExtendedProps, keyof ExtendedProps>> instead of P for the type of passThroughProps.

Nathan Shively-Sanders (@sandersn) I wanted to simplify the example by not involving react but I guess that was bad move :D sorry about that. So here is the React HoC example, with comments:

update 11/22/18:

• added InjectedProps definition
• passThroughProps are casted to P -> {...passThroughProps as P}

import React, { Component } from 'react'

import { Counter } from './counter-render-prop'
import { Subtract } from '../types'

type ExtractFuncArguments<T> = T extends (...args: infer A) => any ? A : never;

// get props that Counter injects via children as a function
// InjectedProps is gonna be:
// { count: number; } & { inc: () => void; dec: () => void; }
type InjectedProps = ExtractFuncArguments<Counter['props']['children']>[0];

// withCounter will enhance returned component by ExtendedProps
type ExtendedProps = { maxCount?: number };

// P is constrained to InjectedProps as we wanna make sure that wrapped component
// implements this props API
const withCounter = <P extends InjectedProps>(Cmp: React.ComponentType<P>) => {
  class WithCounter extends Component<
    // enhanced component will not include InjectedProps anymore as they are injected within render of this HoC and API surface is gonna be extended by ExtendedProps
    Subtract<P, InjectedProps> & ExtendedProps
  > {
    static displayName = `WithCounter(${Cmp.name})`;

    render() {
      const { maxCount, ...passThroughProps } = this.props;

      return (
       // we use Counter which has children as a function API for injecting props
        <Counter>
          {(injectedProps) =>
            maxCount && injectedProps.count >= maxCount ? (
              <p className="alert alert-danger">
                You've reached maximum count! GO HOME {maxCount}
              </p>
            ) : (
              // here cast to as P is needed otherwise compile error will occur
              <Cmp {...injectedProps} {...passThroughProps as P} />
            )
          }
        </Counter>
      );
    }
  }

  return WithCounter;
};

// CounterWannabe implement InjectedProps on it's props
class CounterWannabe extends Component<
  InjectedProps & { colorType?: 'primary' | 'secondary' | 'success' }
> {
  render() {
    const { count, inc, colorType } = this.props;

    const cssClass = `alert alert-${colorType}`;

    return (
      <div style={{ cursor: 'pointer' }} className={cssClass} onClick={inc}>
        {count}
      </div>
    );
  }
}

// if CounterWannabe would not implement InjectedProps this line would get compile error
const ExtendedComponent = withCounter(CounterWannabe);

Martin Hochel (@Hotell) What's the source of Counter? I'm not that familiar with React, so I can't guess how the children of Counter are supposed to be typed.

I read the intro to HOCs and I think I understand what's going on, although I haven't had a chance to play with the whole example without Counter.

1. I still think this is the same issue as posted Veniamin Krol (@vkrol) encountered. That is, Rest<'a', T & { a }> should be assignable to T. I'm not sure why casting injectedProps is needed instead of passThroughProps though.
2. Can't you spread <Cmp {...injectedProps} {...this.props} /> instead? It looks to me the only reason that normal React HOCs use passThroughProps is so that the passthrough component doesn't access the HOC's private state by mistake. But this is not likely in Typescript, because the type checker will complain if CounterWannabe accesses maxCount.
3. Let's file a new issue for this: Generic rest of intersection should be assignable to its type parameter constituents #28636. I think it's a real hole in type simplifications of today.

What's the source of Counter?

Sorry about that, it's not really important I should explicitly provide props in that example.

👉 I've updated previous comment

type State = typeof initialState
type Props = {
  children: (
    props: State & { inc: Counter['handleInc']; dec: Counter['handleDec'] }
  ) => React.ReactChild
  count?: number
} & typeof defaultProps

const initialState = { count: 0 }
const defaultProps = {
  onChange: (value: number) => {}
}
export class Counter extends Component<Props, State> {
  static defaultProps = defaultProps
  render(){ /*...*/ }
}

I'm not sure why casting injectedProps is needed instead of passThroughProps though.

My bad, updated the code above

Can't you spread <Cmp {...injectedProps} {...this.props} /> instead?

will produce the same error

let's file a new issue for this: #28636.
I agree, can you update the code in that issue pls with my fixes ?

Thanks a lot Nathan Shively-Sanders (@sandersn) 💪