-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Object.assign not type checking #11100
Comments
As I explained on IRC, the result of |
I am close to having a PR for spread types, so when those are in,
Right now I am working in the branch |
(also you'll just be able to use the syntax |
@sandersn So with your change, the result of |
Yes, which is not assignable to |
Thanks, that's great @sandersn. I look forward to it! @Arnavion How come this works? const someState: { foo: string } & { foo: number } = { foo: 1 } I think you explained why in IRC, but I'm not sure I completely understand. Are you saying in the above example we're trying to assign |
|
That makes sense, thank you @Arnavion: // number is not assignable to string & number because an arbitrary number is not necessarily also a string.
const x = 'foo' as string & number
const x1: number = x
// string & number is assignable to number because all string & number are also number.
const x2: string & number = 'foo' |
@sandersn, this looks very good! Shouldn't the new type of assign<T, U>(target: T, source: U): T & { ...U } i.e. all type features of target are preserved, with only properties from U tacked on. This covers the very common use case of adding properties to functions. |
@danielearwicker that's not quite right either because if the function already has a property that Can you point me to some source that spreads properties into functions? I'm always interested in collecting uses of the spread type — previously I thought that real-world uses were always with property bags. |
Yes, I see. I haven't been using object spread operator for this purpose (I only use TS) but I have been using Hiding in plain sight, jQuery's I've been working on a strongly-typed composable Redux library where I make augmented functions this way. Although as you can see, I've simply defined my own |
We are looking for a new type construct (mapped types) to address this instead of the spread and rest operators in the type space. we should have updates on this in the next few days. |
I made #12110. This patch makes possible to declare and constrain the parameters 2nd and above. - assign<T, U>(target: T, source: U): T & U;
+ assign<T, U>(target: T, source: U, ...sources: U[]): T & U; (() => {
type State = { foo: string };
const state: State = { foo: 'bar' };
const newState: State = Object.assign<{}, State>({}, state, { foo: 1 }) // error
})(); |
Any updates on this now 2.1 is shipped? |
Now I'm using the following signatures. export function assign<T extends U, U extends object>(target: T, ...sources: Partial<U>[]): T;
export function assign<T extends U, U extends object>(target: object, source: T, ...sources: Partial<U>[]): T;
export function assign<T extends object>(target: T, ...sources: Partial<T>[]): T;
export function assign<T extends object>(target: object, source: T, ...sources: Partial<T>[]): T; |
Subscribing for updates. |
What is the current status on this? |
@metodribic waiting on a spread type, which is waiting on a difference type to make sure JSX Higher-order-component scenarios work. Now that some kind of difference types are expressible using conditional types, the scenario may work. The conversation is happening at #10727. |
This type definition of Object.assign can fixed in now using utility-types's assign definition https://github.com/piotrwitek/utility-types#assign (maybe steal implementations), but we should wait for type spreading features? |
@airtoxin would utility-type's Assign get this example right? type O = {
x: number
y?: number | undefined
m: () => number
}
declare class C {
x: string
y: string
m2: () => string
}
// now Assign<C, O> should be
type Result = {
x: number
y: number | string
m: () => number
} |
Currently it produces
Meaning that it isn't correctly combining optional properties from the right, and isn't correctly discarding class-originating methods, which are on the prototype and will be discarded by Object.assign at runtime. |
@sandersn would your work also fix this example? |
(Reprinting example for ease of reading) const a: { b: number } = { b: 123 };
Object.assign(a, { c: 456 });
a.c = 456; The problem with that example is that we don't model |
A workaround could be:
The trade-off being an extra line of code for type safety. |
Tracking at #10727 |
TS 2.0.3
Given:
I swear this used to work!
The text was updated successfully, but these errors were encountered: