arqex / freezer

A tree data structure that emits events on updates, even if the modification is triggered by one of the leaves, making it easier to think in a reactive way.
MIT License
1.28k stars 56 forks source link

Doesn't work with typescript out of the box #79

Closed qtis closed 8 years ago

qtis commented 8 years ago

due to check if object constructor is Object or Array - Freezer does not work with Typescript.

There are ways to work around this issue by setting this.constructor = Object in typescript class, but looks extremely hackish and would be great to have official typescript support.

todoubaba commented 8 years ago

+1 Freezer skips over typscript class Foo {...}

todoubaba commented 8 years ago

@qtis this.constructor = Object does not work with angular2's ChangeDetection if class Foo contains methods.

arqex commented 8 years ago

Sorry guys I have never used typescript and I don't even understand this problem. How can I fix it? Can you create a PR or tell me what to change?

Cheers

arqex commented 8 years ago

@todoubaba are you using freezer with Angular 2?

I have never done so, and here it is an user asking for some guidance:

https://github.com/arqex/freezer/issues/78

If you have some spare time to explain it, it would be really nice, we are eager to learn :)

todoubaba commented 8 years ago

As @qtis explained above, Freezer only deals with array or Object currently. But we need class to be freezed, such as freeze object foo:

function Foo() {
   this.bar = 'bar'
}
Foo.prototype.f = function() {
}
var foo = new Foo();
todoubaba commented 8 years ago

@arqex You could play with typescript in here: http://www.typescriptlang.org/Playground And paste the following code in the left:

class Foo {
    bar: string = 'bar';

    f() {
    }
}

let foo = new Foo();
kuraga commented 8 years ago

@todoubaba do you mean extending?

class Foo extends Freezer {
  f() {
  }
}

let foo = new Foo();
todoubaba commented 8 years ago

@kuraga I want to freeze foo by let freezer = new Freezer(foo) just like normal object literal.

https://github.com/aerofs/classy-immutable could freeze the instances of ES6 class by extending.

arqex commented 8 years ago

This is an interesting topic.

I can freeze a class instance but from the point of view of freezer it would be seen like an object.

The instance inside freezer would have 2 main problems:

Can you see a solution to these problems?

Instances are like a bag of functions, that handle an internal state. I would store that internal state in freezer as a plain object and reconstruct the instance every time it changes. This would be a solution, but again you shouldn't use the modifier methods of the instance or those changes would keep undetected.

arqex commented 8 years ago

@qtis you could make it work with your hackish solution, so i guess your problem is slightly different.

If you can set your instance prototype to object, why don't make your instance just a plain object?

The main problem here is that freezer aims to store data, and class instances have functions.

todoubaba commented 8 years ago

How could freezer be able of cloning any kind of class instance?

It could call new obj.constructor() to clone, like https://github.com/aerofs/classy-immutable/blob/master/index.js#L63

arqex commented 8 years ago

That would work only for classes that accepts an instance for the constructor:

var bar = 3;
var f = new Foo( bar );
var clone = new Foo(f);
qtis commented 8 years ago

@qtis you could make it work with your hackish solution, so i guess your problem is slightly different.

Our current project is using typescript everywhere and we prototyped freezer with plain objects - work's good. However for consistency, intellisense and transpile warnings/errors it would be much nicer to have all files as typescript instead some as plain js objects just because freezer checks for Object constructor. Our "freezable" calsses are also plain - no constructor override, functions, etc. I guess we could get away with just typescript Interfaces and plain js objects, but having classes makes life easier :)

I don't have typescript implementation knowledge, but in freezer there's check if foo.constructor is Array or Object. Typescript overrides this info and if it would be possible to detect if foo is either Array, Object OR Typescript Class - that'd be perfect.

arqex commented 8 years ago

I have just released a new version that should handle object instances, let me know if it works ok.

todoubaba commented 8 years ago

0.10.0 works on the top level object instance, but still not work with nested object instance and array elements, such as

class Foo { // works
  bar: Bar; // not works
  bars: Bar[] // works for bars, but not for bars[0]
}
arqex commented 8 years ago

Hey progress on this.

@todoubaba

Probably those class attributes are not enumerable in the instance and freezer can't go through parsing them:

http://arqex.com/967/javascript-properties-enumerable-writable-configurable

For the rest, I need to warn that the latest version of freezer, v0.11.0, changes the default behaviour for object instances. They are not parsed anymore by default, if you want freezer to keep parsing your instances you need to initialize your freezer store like this:

var freezer = new Freezer({my: 'data'}, {parseInstances: true});

Otherwise, the instances will be handled like tree leaves.

I think I can close this issue now.

Thanks everybody.