Avoid knock-on refactoring, when a synchronous function suddenly needs an asynchronous value?

Question

While using javascript I keep running into situations, where previously synchronous code suddenly requires a value, that can only be obtained asynchronously.

For instance, I am working on a TamperMonkey script, where I have a function that operates on a string parsed from location.hash. Now I want to change the code to enable persistence across URL changes in a tab, by using the GM_getTab(callback) interface.

Since I need a couple of functions to be executed in unchanged order, a knock-on effect occurs, since I need to await the value and I suddenly end up refactoring several functions along the call-stack into async functions, until I reach a point, where the order needs no longer be guaranteed.

More importantly however, promises needing to be explicitly awaited, can lead to unexpected behavior, when an await is forgotten: E.g. if(condition()) may suddenly always evaluate to true, and 'Hello '+getUserName() may suddenly result in Hello [object Promise].

Is there some way to avoid this "refactoring hell"?

Simplified example

Subsequently, I present a heavily simplified example of what happened: Needing an await down the callstack, while needing to preserve the execution order lead to refactoring all the way up to the event callback updateFromHash.

// -- Before
function updateFromHash(){
    updateTextFromHash();
    updateTitleFromText();
}

function updateTextFromHash(){
    DISPLAY_AREA.innerText = getTextFromHash();
}

// -- After
async function updateFromHash(){
    await updateTextFromHash();
    updateTitleFromText();
}

async function updateTextFromHash(){
    DISPLAY_AREA.innerText = getTextFromHash()
        || await new Promise((accept,reject)=>GM_getTab(accept));        
}

In this case it is relatively simple, but I have previously seen the asynchronicity bubble up much farther in the call stack and cause unexpected behaviour, when an await was missed. The worst case I've seen was, when I suddenly needed the "DEBUG" flag to depend on an asynchronously stored user setting.

Time Constraints

As pointed out by @DavidSampson, it would probably have been better in the example for the functions not to depend on mutable global state in the first place.

However, in practice code is written under time constraints, often by other people, and then you need a "small change" – but if that small change involves asynchrnous data in a formerly synchronous function, it would be desirable to minimize the refactoring effort. The solution needs to work now, cleaning up the design problem may have to wait until the next project meeting.

In the example given, refactoring is feasible, since it is a small, private TamperMonkey script. It ultimately only serves to illustrate the problem, but under commercial-project scenarios, cleaning up the code may not be feasible within a given project scope.

Answer 1

There's a couple of general practices here that would minimize the effect of the problems you are having.

One thing is to try to write code that is not as dependent on happening in a specific order in the first place -- it can lead to a lot of headaches, as you see here.

async function updateFromHash(){
    await updateTextFromHash();
    updateTitleFromText();
}

Here you update some text in a global variable somewhere (more on that later), and then you call a function that goes and looks at that variable, and updates some other variable based on that. Obviously, if one has not finished happening, the other will not work properly.

But what if, instead, you retrieved the asynchronous data in a single place, and then dispatched both the update calls with the data they need as a function argument? Also, you can use .then chaining to deal with asynchronous data without having to make the function asynchronous.

async getHash(){
  return new Promise((accept,reject)=>GM_getTab(accept))
}
function setText(text){
  DISPLAY_AREA.innerText = text;
}
function setTitle(text){
  // make some modifications to the 'text' variable

  TITLE.innerText = myModifiedText // or whatever
}

function updateFromHash(){
  getHash()
     .then(text => {
               setText(text);
               setTitle(text); 
     // You could also call setTitle first, since they aren't dependent on each other
     });
} 

Another improvement you could make is that, broadly speaking, it's often a good idea to keep your functions pure where possible -- the only things that they should modify on are things you pass in as arguments, and the only effects you expect from them are things that they return. Impurities must exist, for various reasons, but try to keep them on the sidelines of your code.

So in your example, you have a function that modifies a global variable, and then another function that presumably goes and looks at that variable, and changes another variable based on that information. This implicitly binds those two variables together, except it's not clear to the casual observer why that should be the case, and so it makes tracing bugs harder. One way you might handle your situation:

function createTitleFromText(text){
  // modify the text passed in to get the title you want
  return myModifiedText;
  // this function is pure
}

function updateContent(text){
  // This is now the ONLY function that modifies state
  // It also has nothing to do with *how* the text is retrieved
  TITLE_EL.innerText = createTitleFromText(text);
  DISPLAY_AREA.innerText = text;
}

async function getTextFromHash(){
  return new Promise((accept,reject)=>GM_getTab(accept))
}

// Then, somewhere else in your code
updateContent(await getTextFromHash());

It also might be a good idea to sprinkle in some object orientedness to this to make it more clear what owns what, and handle some of the ordering for you.

class Content {
  constructor(textEl, titleEl){
    this.textEl = textEl;
    this.titleEl = titleEl;
  }
  static createTitleFromText(text){
    //make modifications
     return myTitle;
  } 
  update(text){
    this.textEl.innerText = text;
    this.titleEl.innerText = Content.createTitleFromText(text);
  }
}

let myContent = new Content(DISPLAY_AREA, TITLE_EL);

// Later

myContent.update(await getTextFromHash());

There's not one right or wrong way, but these are some ideas you can play with.

As for stopping asynchronicity from bubbling up, using .then chaining is probably your best bet. Taking your original example:

// -- After
function updateFromHash(){
    updateTextFromHash()
      .then(updateTitleFromText);

}

async function updateTextFromHash(){
    DISPLAY_AREA.innerText = getTextFromHash()
        || await new Promise((accept,reject)=>GM_getTab(accept));        
}

Now updateFromHash can stay synchronous, but keep in mind that updateFromHash completing does NOT mean that updateTitleFromText is finished, or even that updateTextFromHash is finished. So you'll need to let the asynchronicity bubble up as far as is necessary to deal with any ordered effects.

There is unfortunately no way to synchronously await, due to the JS engine's single threaded nature -- if a synchronous function is waiting for something to finish, nothing else can run.

For particular cases though, you might be able to replicate that functionality synchronously, but it will also involve lots of refactoring.

You could, for example, define a property DISPLAY_AREA.isValid = true, and then in updateTextFromHash

DISPLAY_AREA.isValid = false;
DISPLAY_AREA.innerText = getTextFromHash()
        || await new Promise((accept,reject)=>GM_getTab(accept));
DISPLAY_AREA.isValid = true;

Then, in any code that needs DISPLAY_AREA.innerText, first check to see if that data is valid, and if it isn't, then setTimeout for some period of time and then check again.

Alternatively, you could also define a DISPLAY_AREA.queuedActions = [], and then your other functions can check the validity of DISPLAY_AREA and if it is false, add a callback to the queuedActions. Then updateTextFromHash looks like:

DISPLAY_AREA.isValid = false;
DISPLAY_AREA.innerText = getTextFromHash()
        || await new Promise((accept,reject)=>GM_getTab(accept));
for(var cb of DISPLAY_AREA.queuedActions){
  cb()
}
DISPLAY_AREA.isValid = true;

Of course, in most cases this would just lead to your invalidation logic bubbling up the way async/await does, but in some circumstances it might work out cleanly.