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

refactor: flatten nested if-else blocks #6

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

cades
Copy link

@cades cades commented Jan 10, 2018

apply "return early, avoid else" practice to make code simple and easy to read

@developit
Copy link
Owner

Need to check the effect on filesize, but I'd imagine this is a savings! (due to uglify better optimizing early returns than nested conditionals)

src/index.js Outdated
}

Promise.resolve(() => method.apply(null, data.params))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This creates a Promise with a value resolved to the function itself. See below:
screen shot 2018-01-11 at 10 01 02 am

@developit
Copy link
Owner

developit commented Jan 12, 2018

hey @cades - that nested .then() is important - without it, an exception thrown by the method being called won't be passed back via RPC, it'll just be emitted as an error in the worker.

@cades
Copy link
Author

cades commented Jan 12, 2018

@developit thanks for point that out! Seems my knowledge to Promise is not promising 😂
BTW, if you think it is OK to accept this PR, I'm willing to squash these commits into a single commit before merge.

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

Successfully merging this pull request may close these issues.

2 participants