Synthetic Events get lost through an Action

[eek]

I have a Store Action that gets triggered on an input field onChange (it's a type="file" input), I need the event in order to get the target's values, but triggering a Store Action on onChange gets the event killed on the way.

warning.js:36 Warning: This synthetic event is reused for performance reasons. If you're seeing this, you're accessing the property `target` on a released/nullified synthetic event. This is set to null. If you must keep the original synthetic event around, use event.persist().

Shouldn't actions be synchronous operation? Unless specified otherwise?

[BryanGrezeszak]

I personally agree that actions should be sync by default, unless they have child-actions to perform (like completed and failed, or any other child-actions), in which case then automatically be async.

Logical operation-wise this makes sense to me because there's no reason for most actions without child-actions to be async, and having it async means that you can't just call it and then immediately use the changed values on the next line.

API-wise it also makes sense to me because in many ways the API sorta implies that having child-actions is the de-facto concept of being "async" (for example asyncResult: true strongly implies you're making it async when it was previously sync...when in reality it was async anyway, you're just automatically adding child-actions).

However, that is not the way Reflux has been historically. Historically all actions are async unless you tell it otherwise when you create the action. So it would be enormously breaking of a change to change that, so I've left it be.

The two options of the moment are:

1) Make it a sync action when you define it initially, via sync: true in the definition when you make the action.

2) Any given individual call of an action you can force to be called sync by calling Actions.actionName.trigger() instead of just Actions.actionName() directly. (same goes for triggerAsync to force async for an individual call, btw)

It'll probably stay this way at least for a while just on pure account of how large of a breaking change it would be. But...I do personally agree with you on the subject :)

[BryanGrezeszak]

Hmm...thought about this one for a bit.

There's a LOT of merit in the idea of actions being sync by default unless they have child-actions. But there's also a lot of merit in the idea of NOT wanting to completely change course on such a fundamental thing at such a late juncture.

One thing that's coming to mind is a property that can be used to turn on the idea of sync actions by default for non child-action-having actions. Like Reflux.syncDefault = true;. That way nothing changes unless you purposefully want it to change.

I would like a name that better reflects that it's only simple actions without child-actions that would be made sync by default, though. Not sure how easy that would be to accomplish while remaining short enough to easily remember though. Any naming ideas from anyone?

[eek]

Reflux.syncActions = true; sound ok 😁 Or even the long version Reflux.synchronousActions = true;

Hmm :-?

I would bet that more than 80% of the users think that Actions are synchronous by default. :))

[BryanGrezeszak]

Something like Reflux.actionsWithoutChildrenAreSyncByDefault would be the perfect level of descriptiveness for proper readability...but obviously way too verbose overall.

Reflux.syncActions is a perfect size to easily use/remember...but I worry that it can too easily give the false impression that ALL actions are being made sync, even though actions with children would obviously have to remain async.

I'll try to think of a good middle ground. If not I'll just have to default to Reflux.syncActions or something very similar.

[FlorianWendelborn]

If you think that this should be enabled per default, then why don't we just bump a major version and write this change into a migration guide?

[eek]

I would vote for default being synced. And bumping the major version. I just stumbled upon this sync problem again. Darn.

Also, if we will ever try to push and bring back some Redux users to Reflux, we definitely need sync by default. Redux is sync by default, I think most of the people expect it to be sync.

[BryanGrezeszak]

My general thoughts on bumping major version is that you should have a pretty good reason to do so, because anytime you do something backward-breaking like that you tick off a number of people that don't like change. So the benefit of the backward-breaking change has to outweigh its own cost.

Been debating on whether this sync change balances that equation or not.

...though eek's point about redux users is a good one...I think that tips scales enough to be worth the major change.

I'll start looking at what might need to be done to make actions sync by default (except for actions with child-actions).

[BryanGrezeszak]

Major update to 6.0.0 is done.

Not closing this out quite yet. Would like a little real world testing. Let me know what you guys get for results.

[eek]

It's not something bad, but I noticed that now, linking to a store (that hasn't yet been initiated) in the constructor of a Reflux.Component and also triggering an action from there, no longer works. And it worked before. The action would trigger the function from the store but now the store doesn't exist yet.

Not bad - it was something bad I was doing anyway :))

[BryanGrezeszak]

@eek - the only reason it would have worked before was because of it being async (so the action wasn't actually being called in the constructor, the call was being pushed to the event loop and done after the fact).

If you were to make the action async or do myAction.triggerAsync() specfically in the constructor then you'll likely get the same results as before.

But it seems like a dangerous practice anyway though. I wonder if sometimes it could have race issues where sometimes the component doesn't get mounted by the time the event loop comes around and the store isn't made yet. I guess I don't know if there's ever time between constructor and mounting in react in some situations.

[eek]

Indeed, triggerAsync works there. The problem is when you want to initiate a store with some data that only the component knows. For example, you enter on a page, and you want to have your router to already make a request when initiated for that page that is transmitted via props.

[BryanGrezeszak]

Well, the store is automatically created during componentWillMount.

So there's two main options. Move your action call after that, or create your store before that.

So you could do your action call in the componentWillMount, after you super call. Or do it async to push the action to the event loop like you were doing (though, like I said...be careful with that...I don't think there's any reason React couldn't one day just make there be time between constructor and mount, in which case that would make a race issue).

Or, (I think) the more elegant way to do things, if you know you're gonna use actions before components mount, then you can just not wait for the store to be made automatically, and call Reflux.initStore(MyStoreClass);. You can probably just put it at the end of the store class itself.