omniscientjs / immstruct

Immutable data structures with history for top-to-bottom properties in component based libraries like React. Based on Immutable.js
374 stars 21 forks source link

Be able to dynamically disable/enable changeListeners #25

Closed dashed closed 9 years ago

dashed commented 9 years ago

Use case:

// stop changeListeners from triggering
struct.trigger(false);

// Do multiple modifications.
// 
// Don't trigger swap, add, change, or delete events.
// Also, don't save snapshot into history.
struct.cursor('a').set('foo', 0);
struct.cursor('b').set('bar', 1);

// re-enable changeListeners
struct.trigger(true);

// apply changes
struct.forceHasSwapped();
facundocabrera commented 9 years ago

Could not this be done using the .withMutations method? I mean, if the idea is make bulk update, you already have the mechanism in place, or I'm missing something?

dashed commented 9 years ago

@dendril Not really. I may still want to be able to read values from the structure after updating underlying immutable data structure (bulk or not).

The point is to put changeListener in "noop mode", in Cursor.from(self.current, path, changeListener);.

facundocabrera commented 9 years ago

:+1:

mikaelbr commented 9 years ago

Hmm. I've been thinking about this lately, and I'm getting a bit skeptical to do a state flag like this. You want to limit this "deactivation" to a callstack, right? You wouldn't want this to spread out into parallel components / siblings.

Is the goal to batch changes due to performance? Might it be that the data model is poorly designed (with too dependent data)?

I would think a solution would be something similar to .withMutations like @dendril suggests, but with a "local" cursor:

structure.bulkChange(function (topCursor) {
  topCursor = topCursor.updateIn('child', function (cursor) {
     return cursor.set('foo', 'bar');
  });
  return topCursor.updateIn('anotherChild', function (cursor) {
     return cursor.set('foo', 'bar');
  });
});

This way you can update and access the cursor, without having state handling it and forcing it to happen in one callstack/message queue. But is this a necessary implementation to have? Trying to minimize tha API surface and having only the essential necessities. Thoughts, @Dashed, @torgeir and @dendril ?

facundocabrera commented 9 years ago

@mikaelbr I don't have the situation presented here because I'm pretty new with this project, but that could be easily added by @Dashed using a simple inherits and maybe defining a method to do the struct.removeListener. I guess there is no need to add something extra for now.

mikaelbr commented 9 years ago

Think we have enough patterns for avoiding having to do this? Could this be closed, @Dashed ?

dashed commented 9 years ago

I don't really have an idiomatic pattern for this. But I haven't run into this use case in my own usage.

mikaelbr commented 9 years ago

I mean, having something like deactivate/activate would essentially tightly couple components, and I don't think it is a good API. If one want to do several updates without triggering change you'd do something like:

var newCursor = cursor.update(function (cur) {
  // Do multiple modifications.
  // 
  // Don't trigger swap, add, change, or delete events.
  // Also, don't save snapshot into history.
  cur.set('foo', 0);
  cur.set('bar', 1);
  return cur;
});

This wouldn't allow your state to leak through the system, but isolate it.

dashed commented 9 years ago

Yep. That's what I do most of the time :+1: