Skip to content

Generic type-safe implementation for Object.assign #30481

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

Closed
wants to merge 1 commit into from

Conversation

@typescript-bot
Copy link
Collaborator

It looks like you've sent a pull request to update our 'lib' files. These files aren't meant to be edited by hand, as they consist of last-known good states of the compiler and are generated from 'src'. Unless this is necessary, consider closing the pull request and sending a separate PR to update 'src'.

@msftclas
Copy link

msftclas commented Mar 19, 2019

CLA assistant check
All CLA requirements met.

@typescript-bot
Copy link
Collaborator

It looks like you've sent a pull request to update our 'lib' files. These files aren't meant to be edited by hand, as they consist of last-known good states of the compiler and are generated from 'src'. Unless this is necessary, consider closing the pull request and sending a separate PR to update 'src'.

1 similar comment
@typescript-bot
Copy link
Collaborator

It looks like you've sent a pull request to update our 'lib' files. These files aren't meant to be edited by hand, as they consist of last-known good states of the compiler and are generated from 'src'. Unless this is necessary, consider closing the pull request and sending a separate PR to update 'src'.

It seems that it can be possible to make a type-safe `Object.assign`-like functions with TypeScript, without having to make lots of overrides.

Here's an example on the playground:

typescriptlang.org/play/index.html#src=function%20assign%3CSource%20extends%20%7B%7D%2C%20Result%20extends%20Source%3E(%0D%0A%20%20%20%20target%3A%20Source%2C%0D%0A%20%20%20%20...sources%3A%20Source%5B%5D%0D%0A)%3A%20Pick%3CResult%2C%20keyof%20Source%3E%20%7B%0D%0A%20%20%20%20return%20Object.assign(target%2C%20...sources)%3B%0D%0A%7D%0D%0A%0D%0Alet%20x%20%3D%0D%0A%20%20%20%20assign(%0D%0A%20%20%20%20%20%20%20%20%7B%20hello%3A%20%22world%22%20%7D%2C%0D%0A%20%20%20%20%20%20%20%20%7B%20something%3A%20%22is%20good%22%20%7D%2C%0D%0A%20%20%20%20%20%20%20%20%7B%20%22yes%22%3A%20true%20%7D%2C%0D%0A%20%20%20%20%20%20%20%20%7B%20yes%3A%20%22hello%22%20%7D%0D%0A%20%20%20%20)%3B%0D%0A%0D%0Ax.hello%3B%0D%0Ax.world%3B%20%2F%2F%20error!%0D%0Ax.yes%3B

You can see that the compiler says `x.hello` is fine, and complains about `x.world`
@Schniz Schniz marked this pull request as ready for review March 19, 2019 08:56
@j-oliveras
Copy link
Contributor

Your change don't work correctly (playground):

function assign<Source extends {}, Result extends Source>(
    target: Source,
    ...sources: Source[]
): Pick<Result, keyof Source> {
    return Object.assign(target, ...sources);
}

let y1 = assign(
    () => "",
    {
        version: 1
    }
);

let y2 = Object.assign(
    () => "",
    {
        version: 1
    }
);

y1 (using your change) has an error on version property:

Argument of type '{ version: number; }' is not assignable to parameter of type '() => string'.
  Object literal may only specify known properties, and 'version' does not exist in type '() => string'.

but y2 (using existing Object.assign definition) has no problems.

@RyanCavanaugh
Copy link
Member

It'd be best to start with an issue describing non-working scenarios and outlining tests. We can't really evaluate a PR without first understanding why it's being made. I think the comment from @j-oliveras is instructive - this would likely be a large compat problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants