Skip to content
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

Class.assign() to return an typed Object.assign() #11508

Closed
michaeljota opened this issue Oct 11, 2016 · 8 comments
Closed

Class.assign() to return an typed Object.assign() #11508

michaeljota opened this issue Oct 11, 2016 · 8 comments
Labels
Duplicate An existing issue was already created

Comments

@michaeljota
Copy link

michaeljota commented Oct 11, 2016

Scenario:

Right now Object.assign() doesn't return a typed Object. And as I see it, it should not return one. The Object.assign() should be use as in plain JS, allowing to shallow merge any typed objects.
Instead, I would suggest the use of a .assign(). This assign() should be a method in the Class object, and not in the instance. This would allow to explicit request and be sure you will only retrieve a typed object with the wanted class, in a way that I find easy to remember and logical to reproduce.

Syntactic:

   public static assign(...objs) {
        objs = objs.filter(obj => obj !== {} || obj === null || obj === undefined);
        return objs
            .map(obj => {
// I guess there is a better way to instance an object that assuming and empty constructor.
                let newObj = new T();
                Object.keys(newObj).forEach(key => {
                    if (obj.hasOwnProperty(key)) {
                        newObj[key] = obj[key];
                    };
                });
                return newObj;
            })
            .reduce((previus, current) => {
                return Object.assign(previus, current);
            });
    }

Emit:

This emit something like this (got it from the playground):

   T.assign = function () {
        var objs = [];
        for (var _i = 0; _i < arguments.length; _i++) {
            objs[_i - 0] = arguments[_i];
        }
        objs = objs.filter(function (obj) { return obj !== {} || obj === null || obj === undefined; });
        return objs
            .map(function (obj) {
                var newObj = new T();
                Object.keys(newObj).forEach(function (key) {
                    if (obj.hasOwnProperty(key)) {
                        newObj[key] = obj[key];
                    }
                });
                return newObj;
        })
        .reduce(function (previus, current) {
            return Object.assign(previus, current);
        });
    };

Compatibility:

I just test this in the playground, but this is just plain JS to get a typed object.

Other:

  • Example code: (used to test it in the Playground)
class Greeter {
    greeting: string;
    constructor(message: string) {
        this.greeting = message;
    }
    greet() {
        return "Hello, " + this.greeting;
    }
    public static assign(...objs) {
        objs = objs.filter(obj => obj !== {} || obj === null || obj === undefined);
        return objs
            .map(obj => {
                let newObj = new Greeter('');
                Object.keys(newObj).forEach(key => {
                    if (obj.hasOwnProperty(key)) {
                        newObj[key] = obj[key];
                    }
                });
                return newObj;
            })
            .reduce((previus, current) => {
                return Object.assign(previus, current);
            });
    }
}

let greeter = new Greeter("world");
greeter = Greeter.assign(
    {},
    greeter,
    {
        greeting: 'from Venezuela'
    });

let button = document.createElement('button');
button.textContent = "Say Hello";
button.onclick = function() {
    alert(greeter.greet());
}

document.body.appendChild(button);

I don't see that this can have any performance issues. I search a little before post this, but I have to recognize, but search was not exhaustive. This can help this #11100, and I think this can help this as well #11150, cause the spread can call it's own Class.assign internally.

I really hope you like this. :). Thanks you for reading.

@danielearwicker
Copy link

Take a look at lib.es6.d.ts - it defines Object.assign with proper typing for up to 4 arguments, e.g.

assign<T, U>(target: T, source: U): T & U;

So the returned type will be the intersection of the input types. This is usually a good enough approximation to the type of the resulting object (things get weird if you try to merge a function onto another function).

@michaeljota
Copy link
Author

michaeljota commented Oct 11, 2016

I think what I'm proposing would be like this

assign<T>(original: T, ...sources: any[]): T;

or

assign<T>(target: {}, original: T, ...sources: any[]): T;

or

assign<T>(target: any, ...sources: any[]): T;

So the returned type would be the original type to assign. Right? The behavior that I would like it's giving a type the returned object from an assign() would be that type. Not a intersection, just the original type. I don't know if this is something that TypeScript should do or not, but I think this can solve issues related to unexpected type returning, as I mentioned earlier, cause the programmer would be the one who is explicitly declaring which type it's expected to be giving.

@mhegazy
Copy link
Contributor

mhegazy commented Oct 12, 2016

This is a duplicate of #11100.

@michaeljota, i do not think the proposed declaration matches the semantics of Object.assing. a more accurate declaration would use the spread operator as such:

assign<T, U>(target: T, source: U): { T, ...U };

@mhegazy
Copy link
Contributor

mhegazy commented Oct 12, 2016

closing in favor of #11100.

@mhegazy mhegazy closed this as completed Oct 12, 2016
@mhegazy mhegazy added the Duplicate An existing issue was already created label Oct 12, 2016
@michaeljota
Copy link
Author

I read #11100. I know that plain in JS the desired behavior would be to copy all properties, but in TypeScript, would nice to have something that instead of giving us a intersection of all the types in the assignment, someone could explicitly ask for a giving type. I wasn't mainly asking for Object.assign() to have type checking, cause I doesn't really make sense to me that I have such thing.

I don't know. I see this like an alternative. Anyway, thanks. :).

@kitsonk
Copy link
Contributor

kitsonk commented Oct 12, 2016

You can always assert the generics directly and then it will guard against them:

interface Foo {
    foo: string;
}

interface Bar {
    bar: string;
}

const foobar1 = Object.assign({ foo: 'bar' }, { bar: 2 }); // No error
const foobar2 = Object.assign<Foo, Bar>({ foo: 'bar' }, { bar: 2 }); // Type 'number' is not assignable to type 'string'.

@mhegazy
Copy link
Contributor

mhegazy commented Oct 12, 2016

I read #11100. I know that plain in JS the desired behavior would be to copy all properties, but in TypeScript, would nice to have something that instead of giving us a intersection of all the types in the assignment, someone could explicitly ask for a giving type. I wasn't mainly asking for Object.assign() to have type checking, cause I doesn't really make sense to me that I have such thing.

nothing stops you from changing the declaration of assign locally, or adding a new wrapper for Object.assing that have a different declaration. but I believe the declaration in lib.d.ts should model the runtime behavior as much as possible.

@michaeljota
Copy link
Author

Yeah. I though about it, you are right. I think that what I'm looking changes completely the runtime behavior, and would need additional twerks to make it work. Thank you.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants