getCurrentWindow inconsistent callback behavior

[jleft]

getCurrentWindow(callback, errorCallback) always returns a Window but it may not be ready to use until the callback has been called (observed in OpenFin implementation).

However, sometimes the callback is never called; as a consumer of the API, I don't know when I can safely use the Window object returned by this method.

Annotated code in the API with comments to highlight the issue:

static getCurrentWindow(callback?: (win: Window) => void, errorCallback?: (err?: any) => void) {
  if (currentWindow) {
    // In this branch, callback will never be called,
    // how do I know that currentWindow is ready to use?
    return currentWindow;
  }
  
  // In this branch, I have to wait until callback is called,
  // otherwise currentWindow may not be ready
  currentWindow = new Window(null, callback, errorCallback);
  return currentWindow;
}

[jleft]

I've added some code samples to illustrate the issue - if any further clarification or examples could help, I'm happy to provide.

General issue

// The first time getCurrentWindow is called, I need to wait for the callback to return,
// otherwise the returned window may not be ready to use

var currentWindow = ssf.Window.getCurrentWindow(() => {
  // Safe to use "currentWindow"
});
// Not safe to use "currentWindow" (until the callback is called)


// For any subsequent calls to getCurrentWindow, the callback will never be called,
// but the returned window will be ready to use
var currentWindow = ssf.Window.getCurrentWindow(() => {
  // This will never be called
});
// Safe to use "currentWindow" (as the internally cached window will be returned)

Example use-case

An application could implement its own window chrome; including minimise, maximise, close buttons etc. For example, see StockFlux, in the top-right corner of each window:

Each of the buttons could be its own component and on click could call the corresponding ContainerJS method for that window (e.g. maximise / minimise).

The components could be independent and handle the click events like this:

// Maximise Button Component
// ...
handleClick() {
  const currentWindow = ssf.Window.getCurrentWindow(() => {
    currentWindow.maximize();
  });
}
// ...

// Minimise Button Component
// ...
handleClick() {
  const currentWindow = ssf.Window.getCurrentWindow(() => {
    currentWindow.minimize();
  });
}
// ...

In this case, if the user was to click maximise, then minimise, minimise would not work, as the callback would never be called.

As the order of calls is determined by the user, the only workarounds I can think of is for the application to maintain state somewhere to manage access to the current window - this feels like it should be something that ContainerJS should handle.

Potential Solution

static getCurrentWindow(callback?: (win: Window) => void, errorCallback?: (err?: any) => void) {
  if (currentWindow) {
    callback(currentWindow); // <-- adding this could fix the issue
    return currentWindow;
  }

  currentWindow = new Window(null, callback, errorCallback);
  return currentWindow;
}

[BenLambertNcl]

Maybe if we use your proposed solution, we should remove the return statements? They seem fairly pointless, as the window object is being passed to the callbacks anyway?

[jleft]

Reopening as I can reproduce this issue.

Looks like the original fix was reverted in #342.