Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fluxible] context.executeAction(action, null, callback) invokes the callback before the async action finishes #593

Open
Kailang opened this issue Jun 27, 2018 · 1 comment

Comments

@Kailang
Copy link

Kailang commented Jun 27, 2018

When using babel to compile ES2015 down, there is a bug that causes the actionContext.executeAction(action, null, callback) to invoke the callback before the async action finishes.
This bug causes the executeMultiple() to ignore the dependency chain and invokes the action before its dependencies are fulfilled.

Expected behavior

Consider the following 2 actions:

function loadView(context, params = {}, done) { }
function initViewPage(context, params, done) {
    actionContext.executeAction(loadView, () => {
        console.log('This should not be printed.');
    });
}

When executing initViewPage() action, the console log should not be printed since loadView() action never finishes.

Actual behavior

The console log is printed without waiting for the loadView() action to finish(, which should never happen).

Information about the Issue

In:
https://github.com/yahoo/fluxible/blob/master/packages/fluxible/utils/callAction.js#L29

                var syncResult = action(actionContext, payload, function (err, result) {
                    if (err) {
                        reject(err);
                    } else {
                        resolve(result);
                    }
                });
                if (isPromise(syncResult)) {
                    syncResult.then(resolve, reject);
                } else if (action.length < 3) {
                    resolve(syncResult);
                }

Especially, in:

                if (action.length < 3) {
                    resolve(syncResult);
                }

Here, the callback for any action is directly invoked if the parameter count for the action is less than 3.
Since we are using babel to compile the ES2015 down, we cannot rely on the <Func>.length property to determine if we have given the action a callback.
For example:

function loadViews(context, params = {}, done) { }

loadViews.length will be 1 because it is compiled into:

function loadViews(context) {
  var params = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : {};
  var done = arguments[2];
}

by babel.
A quick way to avoid this bug is to not use the default parameters syntax in the action definition if you are using babel.

Steps to reproduce the behavior

Try the code in the first section in any babel compiled code.

@Kailang Kailang changed the title [ES2015] context.executeAction(action) invokes the action's callback before the action finishes [fluxible] context.executeAction(action) invokes the action's callback before the action finishes Jun 27, 2018
@Kailang Kailang changed the title [fluxible] context.executeAction(action) invokes the action's callback before the action finishes [fluxible] context.executeAction(action, null, callback) invokes the callback before the async action finishes Jun 27, 2018
@mridgway
Copy link
Collaborator

I believe this is the same as #444. This can't be fixed without a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants