I am thinking could it be a good approach to always return promises in functions in JavaScript?
Let's imagine the case where we have a function that validates a username. A master function simply utilises 2 other functions that perform different checks.
Please note, all functions names are just examples.
// Returns a boolean
function validateUsername (username) {
return validateUsernameFormat(username) &&
isUsernameReserved(username);
}
// Returns a boolean
function validateUsernameFormat (username) {
return typeOf(username) === 'string' &&
username.match(/^\[a-z0-9]{8,20}$/);
}
// Returns a boolean
function isUsernameNotReserved (username) {
return ['igor', 'kristina'].indexOf(username) === -1;
}
Now let's imagine we augment our validation in the future by calling API to check if a given username already exists in our database.
// Now returns a promise
function isUsernameNotReserved (username) {
return API.checkIfUserNameAlreadyExists(username);
}
This would mean we will also now have to change the master validateUsername function since it now also needs to return promise. This would also probably mean we will have to modify all functions that use validateUsername function.
But what If we had all function in promises from scratch?
Option A - All functions return promises
// Returns a promise
function validateUsername (username) {
return validateUsernameFormat(username)
.then(() => {
return isUsernameReserved(username);
});
}
// Returns a promise
function validateUsernameFormat (username) {
return (
typeOf(username) === 'string' && username.match(/^\[a-z0-9]{8,20}$/) ?
Promise.resolve() : Promise.reject()
);
}
// Returns a promise
function isUsernameNotReserved (username) {
return (
['igor', 'kristina'].indexOf(username) === -1 ?
Promise.resolve() : Promise.reject()
);
}
Now if we want to augment isUsernameNotReserved with asynchronous API call, we don't need to change anything else.
Option B - Only functions calling another functions return promises
Also, another option would be write functions in promises that call another functions. In that case, only validateUsername should be written as a promise from scratch.
Is this a good approach? What could be the drawbacks apart from performance?
Upd: ran a simple performance test and though running consequent promises is slower, it practically should not make any difference since running 100000 consequent functions takes ~200ms, while running 1000 takes ~3ms in Chrome. Fiddle here https://jsfiddle.net/igorpavlov/o7nb71np/2/
Should I always return promises in all functions in JavaScript?
No.
If you have a function that performs an asynchronous operation or may perform an asynchronous operation, then it is reasonable and generally good design to return a promise from that function.
But, if your function is entirely synchronous and no reasonable amount of forethought thinks this will sometime soon contain an asynchronous operation, then there are a bunch of reason why you should not return a promise from it:
Asynchronous code writing and testing is more complicated than synchronous code writing and testing. So, you really don't want to make code harder to write and test than it needs to be. If your code can be synchronous (and just return a normal value), then you should do so.
Every .then() handler gets called on the next tick (guaranteed asynchronously) so if you take a whole series of synchronous operations and force each function to wait until the next tick of the event loop, you're slowing down code execution. In addition, you're adding to the work of the garbage collector for every single function call (since there's now a promise object associated with every single function call).
Losing the ability to return a normal value from a synchronous function is a huge step backwards in the language tools that you can conveniently use to write normal code. You really don't want to give that up on every single function.
Now if we want to augment isUsernameNotReserved with asynchronous API call, we don't need to change anything else.
A good API design would anticipate whether an asynchronous API is relevant or likely useful in the near future and make the API async only in that case. Because asynchronous APIs are more work to write, use and test, you don't want to unnecessarily just make everything async "just in case". But, it would be wise to anticipate if you are likely to want to use an async operation in your API in the future or not. You just don't want to go overboard here. Remember, APIs can be added to or extended in the future to support new async things that come along so you don't have to over complicate things in an effort to pre-anticipate everything that ever might happen in the future. You want to draw a balance.
"Balance" seems like the right word here. You want to balance the likely future needs of your API with developer simplicity. Making everything return a promise does not draw a proper balance, as it chooses infinite flexibility in the future while over complicating things that don't need to be as complicated.
In looking at a couple of your particular examples:
validateUsernameFormat() does not seem likely to ever need to be asynchronous so I see no reason for it to return a promise.
isUsernameNotReserved() does seem like it may need to be asynchronous at some point since you may need to look some data up in a database in order to determine if the user name is available.
Though, I might point out that checking if a user name is available before trying to just create it can create a race condition. This type of check may still be used as UI feedback, but when actually creating the name, you generally need to create it in an atomic way such that two requests looking for the same name can't collide. Usually this is done by creating the name in the database with settings that will cause it to succeed if the name does not already exist, but fail if it does. Then, the burden is on the database (where it belongs) to handle concurrency issues for two requests both trying to create the same user name.
