Mobx: Request to Un-deprecate asStructure.

Created on 12 Jan 2017  Â·  29Comments  Â·  Source: mobxjs/mobx

As discussed on Twitter, I'm really hoping to have transaction and asStructure put back into mobx. We have lots and lots of places in code where we do things like this

@observable filters = asStructure({});

and this

function syncHashWatcher(){
    let currentHashInfo = hashUtility.getCurrentHashInfo();

    transaction(() => {
        currentHash.module = currentHashInfo.module;
        currentHash.submodule = currentHashInfo.submodule;
        currentHash.parameters = currentHashInfo.parameters;
    });
}

Edit - transaction does seem to have a direct, one-to-one conversion, so I can probably live without that. Still worried about asStructure though.

Edit 2 - we're also wondering if the deepEqual method you're using with asStructure might be able to be exported?

Most helpful comment

@arackaf, check this out, I've just made a simple package to replace those automatically: mobx-transaction-migration
@mweststrate may be it would make sense to try migrating other changes in the same fashion?

All 29 comments

With transaction, is there a reason you can't use runInAction? I just
replaced all our transactions the other day, seems to work fine.

On Thu, Jan 12, 2017 at 3:50 PM, Adam Rackis notifications@github.com
wrote:

As discussed on Twitter, I'm really hoping to have transaction and
asStructure put back into mobx. We have lots and lots of places in code
where we do things like this

@observable filters = asStructure({});

and this

transaction(() => {
//callback which modified multiple observables
});

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/751, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAIrciDuePoDsdQROf_t5HNT1Jgql1-Zks5rRqA4gaJpZM4LiRtp
.

--
-Matt Ruby-
[email protected]

@mweststrate Are there any differences whatsoever between the two? runInAction appears to not be in the docs.

@arackaf it's sneaky in the docs: https://mobx.js.org/refguide/action.html AFAIK the only dif is now you can name and scope if you'd like. Both of those params are optional.

Looks like it passes through the return value, as well

console.log(runInAction(() => 1));

logs 1.

@mweststrate - any differences at all??? I can prolly live without transaction.

Thinking asStructure would definitely still add value, if that could make it back.

Thinking about it - and looking things over some more - there's a TON of places I'll need to change, which always makes me queasy.

If adding transaction back, too, wouldn't be difficult, I think those extra 10 or so bytes might be worth it :)

Plus, it's still in your docs, so that'd be one less thing for you to do :-D

@arackaf, check this out, I've just made a simple package to replace those automatically: mobx-transaction-migration
@mweststrate may be it would make sense to try migrating other changes in the same fashion?

PS: don't forget to backup!

Incredibly helpful, everyone, thanks a ton. @andykog I just toughed out the changes - we'll be testing widely after this upgrade, anyway, and this is a relatively safe change.

The more disconcerting use case is asStructure. We've used that in a number of places to do things like

@observable filters = asStructure({});

or

@observable labels = asStructure([]);

and then reference filters in an autorun, and only be notified if the newly assigned value differed shallowly (which it often didn't).

Is there a direct one-to-one replacement for this? I checked the docs but didn't see anything, though I could have easily missed it. The reference to asStructure in the docs implied this was frequently used for immutable data, which is interesting because that wasn't really our goal.

if the newly assigned value differed shallowly (which it often didn't).

I guess you mean 'deeply'?

@mweststrate Are there any differences whatsoever between the two? runInAction appears to not be in the docs.

Oops, will doublecheck!

Yes, there are differences: 1. transaction fired transaction event in the mobx spy, and runInAction fires action instead. (This is probably not relevant). 2. runInAction also applies untracked, which transaction didn't. The only case where you could note that difference is when using transaction inside a computed value or reaction. But those aren't useful construction as since 2.4 all reactions apply transactions automatically.

Yeah - meant deeply.

And I did surmise that difference, where transaction isn't untracked - can't imagine a transaction where you'd want tracking happening.

But those aren't useful construction as since 2.4 all reactions apply transactions automatically.

@mweststrate are you saying

autorun(() => {
   let val = this.observableValue;
   this.x = val;
   this.y = val + 1;
});

Will automatically batch the updates to x and y into one update?

@arackaf yes

Other question: when using deep comparison, should the value still be converted to an observable or not? I guess so, so that you can later change individual fields of the observables?

@arackaf could you test [email protected], (@)observable.structurallyCompare should do the trick (I hope this name is a bit more clear compared to asStructure ? It deeply converts to structurally comparing observables, which is the same behavior as in MobX 2

@mweststrate I'll get on that tonight - thanks so much! (the one damn GitHub notification I would have liked to get on my real email, and not my spam account :-| )

The name is much, much clearer. So that, I assume, will be the new asStructure? Would it be hard or un-desired to keep asStructure as an alias for compat?

A

@arackaf just curious, can you give more background on those structure examples? The UIs I'm imagining for the filter/label can be decomposed into observable + computed, so I'm wondering what I'm missing...

@mweststrate Done and done. And happy.

I take it back, @observable.structurallyCompare foo = {a: 1, b: 2}; is so, so much more clear than the @observable foo = asStructure({a: 1, b: 2}); that came before it. I don't even want the old version in my code now. No argument from me if you choose to keep it for compat, but it's definitely not at my request :)

And yeah, I confirmed that autorun batches updates, as though in a transaction, so cool - good to know!

Will this be slated for a release soon?

Thanks a ton for all you do!

Hmm... would computed.structurallyCompare then also be a better name? and structurallyCompare as option to reaction for consistency?

@spion hard to really explain - every project is different. For us, with all we have set up, it happens to be convenient to just re-assign observables, and have change trackers only run if the new value is structurally different than the old.

@mweststrate it all depends on whether you prefer short names or long names, but yes I think things should be consistent 😀

FWIW Here are my thoughts on it...

Lets see how it looks:

class User {
  @observable firstName = ''

  @observable.ref lastName = ''

  @observable.structurallyCompare interests = []

  @computed.structurallyCompare get nameData() {
    return {first: firstName, last: lastName}
  }
}

vs

class User {
  @observable firstName = ''

  @observable.ref lastName = ''

  @observable.struct interests = []

  @computed.struct get nameData() {
    return {first: firstName, last: lastName}
  }
}

Arguments for the long name:

  • Its absolutely clear what it means - zero learning curve (well almost - you need to know what structural comparisons are)
  • The name "struct" might be confusing when its a structurally compared array, since thats not a "struct"
  • Consistency is less important when the name makes things absolutely clear

Arguments for the short name:

  • After getting familiar with mobx, its easier to scan in code.
  • It reads pretty well: "class User contains an observable firstName, an observable by reference lastName, an observable by structure list of interests, and a computed structure containing the name data"
  • Its a bit more consistent with ref unless that changes to referentiallyCompare

Naming things is hard! Maybe byRef, byStruct ? No idea.

Yikes - @observable.struct - that would confuse me a great deal if I were new to MobX.

struct to me immediately conveys "value type" which of course JS doesn't have.

That's likely just baggage from the languages I come from, and probably differs for other people, but for such a massively popular project like MobX, I'd recommend the longer and clearer name.

@arackaf I agree its less newbie-friendly, but it makes sense if you squint a bit: "observed as a value-type". The other option being @observable.ref which is "observed as a reference type"

It deeply converts to structurally comparing observables, which is the same behavior as in MobX 2

Maybe it's overkill, but shouldn't be the comparator option available for all modifiers...?

@observable.compareAsStruct // convert deeply, but compare as struct
@observable.deep.compareAsStruct // same 
@observable.ref.compareAsStruct // don't convert, but compare as struct
@observable.shallow.compareAsStruct // convert shallowly, but compare as struct               

// Other silly ideas
@observable.comparator.struct
@observable.comparator(myOwnComparator)

EDIT: Maybe @observable.ref.compareAsStruct can come in handy when working with some graphic/math lib?

@urugator @arackaf I agree, in combination with refs it also seems pretty powerful. Proposed changelog, before continuing :).

Didn't add shallow.compareAsStruct, seems a half arsed in between version. (Contra example by @urugator in 3.. 2... 1..)


Re-introduced structural comparison. Seems we couldn't part with it yet :). So the following things have been added:

  • structurallyCompare option to reaction (alias for compareStructural, to get more consistency in naming)
  • computed.structurallyCompare as alias for computed.struct, to get more consistency in naming
  • observable.structurallyCompare, as alias for observable.deep.structurallyCompare
  • observable.deep.structurallyCompare: Only stores a new value and notify observers if the new value is not structurally the same as the previous value. Beware of cycles! Converts new values automatically to observables (like observable.deep)
  • observable.ref.structurallyCompare: Only stores a new value and notify observers if the new value is not structurally the same as the previous value. Beware of cycles! Doesn't convert the new value into observables.

About the naming, I do like compareAsStruct quite a lot, what do you guys think? @urugator @arackaf @spion?

@mweststrate Love it. One small question though:

Converts new values automatically to observables (like observable.deep)

Was that the same behavior as asStructure? So if I'm happy just storing dumb objects in my observable, and merely want to be notified when the structure itself changes, I'd use observable.ref.structurallyCompare ?

Lastly, any luck getting that deep compare method exposed publicly?

@mweststrate might I suggest structurallyCompared for grammatical consistency? IMO it should either be @observe.structurallyCompare (imperative) or @observable.structurallyCompared (declarative), mixing the two feels weird. Nitpicky, I know!

p.s. I value brevity over newbie clarity, because it takes 10 minutes to learn what the concept means, and then you'll be looking at it for months - so I like to optimise for the second part. But It doesn't seem to be used super often, so the additional chattiness wont be noticeable 99% of the time. So its all good!

edit: @observable.comparedAsStruct also works.

shallow - if I will ever use ref.compareAsStruct lets say for storing points or vectors, then there is good chance I will also need to store an (observable) array of points/vectors...?

naming - I generally try to avoid shifting words from their original part of speech, so I am not a fan of structurallyCompare. I also don't like the order of words , I would prefer compareStructurally.
I woudn't mind compareAsStruct, compareStruct, structComparator or simply struct (to note the prop is handled as "value type") or maybe even valueType or asValueType
EDIT: Actually I think that struct makes more sense in the context of current deep/shallow/ref, do you remeber asStruct/asMap/asSomething ? we moved away from these. We could also say it specifies the type of the prop, not comparator (used comparator can be considered just as a consequence... same as we don't use convertDeeply or deepConverter, but specify observable type)

@urugator I concur, struct is simple and rememberable and consistent. Let's go for that!

@mweststrate sounds great - I really need to dive into the docs again - it seems you guys added tons of new options / features. Great work

Released 3.0.2. Have fun!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Keats picture Keats  Â·  45Comments

AriaFallah picture AriaFallah  Â·  63Comments

mweststrate picture mweststrate  Â·  61Comments

sonaye picture sonaye  Â·  70Comments

flipjs picture flipjs  Â·  31Comments