search

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

Some questions and notes #68

Open kuraga opened 8 years ago

kuraga commented 8 years ago

1) Tag 1c17fe3743e45bf7934a585ab11d202b2741c992 as v0.9.4, please.

2) Propose to use ===/!== instead of ==/!=.

3) Same for tests: strictEqual/notStrictEqual instead of equal/notEqual.

4) Is this assert.deepEqual(data, example); instead?

5) What does remove returns being called with unexistant key(s)? Is it fixed in tests?

6) Listener tests: I don't see tests about triggering callbacks on exact .remove call.

7) Listener tests: I don't see tests about .trigger function.

8) Listener tests: "Listen by several callbacks" test is omitted.

9) Listener tests: Will callback triggered if we try to remove non-existant property?

10) Listener tests: Will callback triggered if we try to update a property but with the same value?

11) Listener tests: ".unsubscribe doesn't touch other callbacks" test is omitted.

12) Listener tests: What's the behavior if we pass non-existant callback to .unsubscribe?

13) Could we merge (some parts of) freezer-spec and hash-spec?

@arqex , thanks! (Note: I've read freezer-spec, hash-spec and listeners-spec only. I use them here.)

arqex commented 8 years ago

Thanks for your list @kuraga 1) I skipped that buggy version in github since I have released the 0.9.5. 2) I merged your PR for this. 3 & 4) I had problems with strictEquals in some tests made for the browser that are outdated now. It would be great to make the test pass again in browser so maybe should be nice to use some assertion library that can be use there too. 5) remove should return always the object where the attributes must be taken off. If the key is not there nothing is removed and no error is raised, but the node where the remove was called is replaced in freezer by a new object. This behaviour is different of what we have in set, there is no change happens freezer's state is not modified at all. Maybe we should modify this remove behavior to match set one. 6-12) Testing and testing.... nobody like testing, I am human and I don't like it either :dancer: . But it is true that if we have a wider test suite we could make bigger changes in the library being confident nothing will get broken. 13) I merge some of your tests, and I hope I can merge others soon.

By the way, rotor.js!!! What a nice name for a new framework!! I'd like to see it working, do you have some demo online??

Cheers, Javi

kuraga commented 8 years ago

@arqex big thanks for reply!

2-4) Note problem is wider: not only in pointed places (you know it :smile:). 5) Cool! Document and/or change. 6-12) Ok. I've made all of these tests but I use other test and code style.

13) I meant another one. Seems like freezer-spec and hash-spec have some common tests. But, maybe, they are different... For root of freezer and nested object... Hm...

Coolness of RotorJS is its modularity. Some of FreezerJs tests are like a standard but you may use other library (I think even mutable) for model (FreezerJs Model file).

The same for other components such as application loop. You may use (virtual) DOM library or make Canvas support (now I support virtual-dom only).

There is an example. I'm creating online demo (another maybe, I think ToDoMVC after stabilizing of API. But coolness is code not possibilities (they are minimal).

Big thanks! Feel free to close.