Convert bindAll to an async function (#3904)

Co-authored-by: Carson <cpsievert1@gmail.com>
Co-authored-by: cpsievert <cpsievert@users.noreply.github.com>

.eslintrc.yml

@@ -15,6 +15,11 @@ parser: '@typescript-eslint/parser'
 parserOptions:
   ecmaVersion: 2018
   sourceType: module
+  project:
+    - './tsconfig.json'
+ignorePatterns: # mirrors tsconfig.json's exclude
+    - '**/__tests__'
+    - '**/*.d.ts'
 plugins:
   - '@typescript-eslint'
   - prettier
@@ -70,6 +75,10 @@ rules:
 
   "@typescript-eslint/consistent-type-imports":
     - error
+
+  "@typescript-eslint/no-floating-promises":
+    - error
+
   "@typescript-eslint/naming-convention":
     - error
 

NEWS.md

@@ -1,5 +1,9 @@
 # shiny (development version)
 
+## Possibly breaking changes
+
+* Closed #3899: The JS functions `Shiny.renderContent()` and `Shiny.bindAll()` are now asynchronous. These changes were motivated by the recent push toward making dynamic UI rendering asynchronous (and should've happened when it was first introduced in Shiny v1.7.5). The vast majority of user code using these functions should continue to work as before, but some code may break if it relies on these functions being synchronous (i.e., blocking downstream operations until completion). In this case, consider `await`-ing the downstream operations (or placing in a `.then()` callback). (#3929)
+
 ## New features and improvements
 
 * Updated `selectizeInput()`'s selectize.js dependency from v0.12.4 to v0.15.2. In addition to many bug fixes and improvements, this update also adds several new [plugin options](https://selectize.dev/docs/demos/plugins). (#3875)

srcts/src/bindings/input/selectInput.ts

@@ -146,6 +146,7 @@ class SelectInputBinding extends InputBinding {
       selectize.settings.load = function (query: string, callback: CallbackFn) {
         const settings = selectize.settings;
 
+        /* eslint-disable @typescript-eslint/no-floating-promises */
         $.ajax({
           url: data.url,
           data: {
@@ -309,6 +310,7 @@ class SelectInputBinding extends InputBinding {
     const binding = $el.data("shiny-input-binding");
     if (binding) shinyUnbindAll($el.parent());
     const control = $el.selectize(options)[0].selectize as SelectizeInfo;
+    /* eslint-disable @typescript-eslint/no-floating-promises */
     if (binding) shinyBindAll($el.parent());
     return control;
   }

srcts/src/file/fileProcessor.ts

@@ -180,6 +180,7 @@ class FileUploader extends FileProcessor {
   onFile(file: File, cont: () => void): void {
     this.onProgress(file, 0);
 
+    /* eslint-disable @typescript-eslint/no-floating-promises */
     $.ajax(this.uploadUrl, {
       type: "POST",
       cache: false,

srcts/src/shiny/bind.ts

@@ -140,14 +140,14 @@ function bindInputs(
   return inputItems;
 }
 
-function bindOutputs(
+async function bindOutputs(
   {
     sendOutputHiddenState,
     maybeAddThemeObserver,
     outputBindings,
   }: BindInputsCtx,
   scope: BindScope = document.documentElement
-): void {
+): Promise<void> {
   const $scope = $(scope);
 
   const bindings = outputBindings.getBindings();
@@ -184,7 +184,7 @@ function bindOutputs(
 
       const bindingAdapter = new OutputBindingAdapter(el, binding);
 
-      shinyAppBindOutput(id, bindingAdapter);
+      await shinyAppBindOutput(id, bindingAdapter);
       $el.data("shiny-output-binding", bindingAdapter);
       $el.addClass("shiny-bound-output");
       if (!$el.attr("aria-live")) $el.attr("aria-live", "polite");
@@ -270,11 +270,11 @@ function unbindOutputs(
 
 // (Named used before TS conversion)
 // eslint-disable-next-line @typescript-eslint/naming-convention
-function _bindAll(
+async function _bindAll(
   shinyCtx: BindInputsCtx,
   scope: BindScope
-): ReturnType<typeof bindInputs> {
-  bindOutputs(shinyCtx, scope);
+): Promise<ReturnType<typeof bindInputs>> {
+  await bindOutputs(shinyCtx, scope);
   return bindInputs(shinyCtx, scope);
 }
 function unbindAll(
@@ -285,10 +285,13 @@ function unbindAll(
   unbindInputs(scope, includeSelf);
   unbindOutputs(shinyCtx, scope, includeSelf);
 }
-function bindAll(shinyCtx: BindInputsCtx, scope: BindScope): void {
+async function bindAll(
+  shinyCtx: BindInputsCtx,
+  scope: BindScope
+): Promise<void> {
   // _bindAll returns input values; it doesn't send them to the server.
   // Shiny.bindAll needs to send the values to the server.
-  const currentInputItems = _bindAll(shinyCtx, scope);
+  const currentInputItems = await _bindAll(shinyCtx, scope);
 
   const inputs = shinyCtx.inputs;
 

srcts/src/shiny/index.ts

@@ -111,6 +111,7 @@ function setShiny(windowShiny_: Shiny): void {
     // Init Shiny a little later than document ready, so user code can
     // run first (i.e. to register bindings)
     setTimeout(function () {
+      /* eslint-disable @typescript-eslint/no-floating-promises */
       initShiny(windowShiny);
     }, 1);
   });

srcts/src/shiny/init.ts

@@ -28,7 +28,7 @@ import { registerNames as singletonsRegisterNames } from "./singletons";
 import type { InputPolicyOpts } from "../inputPolicies/inputPolicy";
 
 // "init_shiny.js"
-function initShiny(windowShiny: Shiny): void {
+async function initShiny(windowShiny: Shiny): Promise<void> {
   setShinyObj(windowShiny);
   const shinyapp = (windowShiny.shinyapp = new ShinyApp());
 
@@ -95,8 +95,8 @@ function initShiny(windowShiny: Shiny): void {
     };
   }
 
-  windowShiny.bindAll = function (scope: BindScope) {
-    bindAll(shinyBindCtx(), scope);
+  windowShiny.bindAll = async function (scope: BindScope) {
+    await bindAll(shinyBindCtx(), scope);
   };
   windowShiny.unbindAll = function (scope: BindScope, includeSelf = false) {
     unbindAll(shinyBindCtx(), scope, includeSelf);
@@ -146,7 +146,7 @@ function initShiny(windowShiny: Shiny): void {
   // have a reference to the DOM element, which would prevent it from being
   // GC'd.
   const initialValues = mapValues(
-    _bindAll(shinyBindCtx(), document.documentElement),
+    await _bindAll(shinyBindCtx(), document.documentElement),
     (x) => x.value
   );
 

srcts/src/shiny/initedMethods.ts

@@ -55,8 +55,8 @@ function setShinyUser(user: string): void {
 function shinyForgetLastInputValue(name: string): void {
   validateShinyHasBeenSet().forgetLastInputValue(name);
 }
-function shinyBindAll(scope: BindScope): void {
-  validateShinyHasBeenSet().bindAll(scope);
+async function shinyBindAll(scope: BindScope): Promise<void> {
+  await validateShinyHasBeenSet().bindAll(scope);
 }
 function shinyUnbindAll(scope: BindScope, includeSelf = false): void {
   validateShinyHasBeenSet().unbindAll(scope, includeSelf);
@@ -65,8 +65,11 @@ function shinyInitializeInputs(scope: BindScope): void {
   validateShinyHasBeenSet().initializeInputs(scope);
 }
 
-function shinyAppBindOutput(id: string, binding: OutputBindingAdapter): void {
-  shinyShinyApp().bindOutput(id, binding);
+async function shinyAppBindOutput(
+  id: string,
+  binding: OutputBindingAdapter
+): Promise<void> {
+  await shinyShinyApp().bindOutput(id, binding);
 }
 
 function shinyAppUnbindOutput(

srcts/src/shiny/reactlog.ts

@@ -50,6 +50,7 @@ function initReactlog(): void {
       window.escape(shinyAppConfig().sessionId);
 
     // send notification
+    /* eslint-disable @typescript-eslint/no-floating-promises */
     $.get(url, function (result: "marked" | void) {
       if (result !== "marked") return;
 

srcts/src/shiny/reconnectDialog.ts

@@ -22,7 +22,7 @@ function updateTime(reconnectTime: number): void {
   }, 1000);
 }
 
-function showReconnectDialog(delay: number): void {
+async function showReconnectDialog(delay: number): Promise<void> {
   const reconnectTime = new Date().getTime() + delay;
 
   // If there's already a reconnect dialog, don't add another
@@ -34,7 +34,7 @@ function showReconnectDialog(delay: number): void {
   const action =
     '<a id="shiny-reconnect-now" href="#" onclick="Shiny.shinyapp.reconnect();">Try now</a>';
 
-  showNotification({
+  await showNotification({
     id: "reconnect",
     html: html,
     action: action,

srcts/src/shiny/render.ts

@@ -35,7 +35,7 @@ import type { WherePosition } from "./singletons";
 // Render HTML in a DOM element, add dependencies, and bind Shiny
 // inputs/outputs. `content` can be null, a string, or an object with
 // properties 'html' and 'deps'.
-async function renderContentAsync(
+async function renderContent(
   el: BindScope,
   content: string | { html: string; deps?: HtmlDep[] } | null,
   where: WherePosition = "replace"
@@ -62,7 +62,7 @@ async function renderContentAsync(
 
   if (where === "replace") {
     shinyInitializeInputs(el);
-    shinyBindAll(el);
+    await shinyBindAll(el);
   } else {
     const $parent = $(el).parent();
 
@@ -75,52 +75,26 @@ async function renderContentAsync(
       }
     }
     shinyInitializeInputs(scope);
-    shinyBindAll(scope);
+    await shinyBindAll(scope);
   }
 }
 
-function renderContent(
+// This function was introduced in v1.7.5, then deprecated in v1.8.0 once we
+// realized renderContent() should really be async, partly as a consequence of
+// bindAll() wanting to be async, as well as there being (seemingly) a small
+// amount of risk in breaking existing behavior. We haven't (yet) decided to do
+// something similar with renderDependencies()/renderHtml() since there is more
+// obvious risk with doing so (e.g., it's very likely a lot of user code is
+// relying on dependencies to be rendering syncronously)
+async function renderContentAsync(
   el: BindScope,
   content: string | { html: string; deps?: HtmlDep[] } | null,
   where: WherePosition = "replace"
-): void {
-  if (where === "replace") {
-    shinyUnbindAll(el);
-  }
-
-  let html = "";
-  let dependencies: HtmlDep[] = [];
-
-  if (content === null) {
-    html = "";
-  } else if (typeof content === "string") {
-    html = content;
-  } else if (typeof content === "object") {
-    html = content.html;
-    dependencies = content.deps || [];
-  }
-
-  renderHtml(html, el, dependencies, where);
-
-  let scope: BindScope = el;
-
-  if (where === "replace") {
-    shinyInitializeInputs(el);
-    shinyBindAll(el);
-  } else {
-    const $parent = $(el).parent();
-
-    if ($parent.length > 0) {
-      scope = $parent;
-      if (where === "beforeBegin" || where === "afterEnd") {
-        const $grandparent = $parent.parent();
-
-        if ($grandparent.length > 0) scope = $grandparent;
-      }
-    }
-    shinyInitializeInputs(scope);
-    shinyBindAll(scope);
-  }
+): Promise<void> {
+  console.warn(
+    "renderContentAsync() is deprecated. Use renderContent() instead."
+  );
+  await renderContent(el, content, where);
 }
 
 // =============================================================================

srcts/src/shiny/shinyapp.ts

@@ -237,6 +237,9 @@ class ShinyApp {
         socket.send(msg as string);
       }
 
+      // This launches the action queue loop, which just runs in the background,
+      // so we don't need to await it.
+      /* eslint-disable @typescript-eslint/no-floating-promises */
       this.startActionQueueLoop();
     };
     socket.onmessage = (e) => {
@@ -507,12 +510,16 @@ class ShinyApp {
     return value;
   }
 
-  bindOutput(id: string, binding: OutputBindingAdapter): OutputBindingAdapter {
+  async bindOutput(
+    id: string,
+    binding: OutputBindingAdapter
+  ): Promise<OutputBindingAdapter> {
     if (!id) throw "Can't bind an element with no ID";
     if (this.$bindings[id]) throw "Duplicate binding for ID " + id;
     this.$bindings[id] = binding;
 
-    if (this.$values[id] !== undefined) binding.onValueChange(this.$values[id]);
+    if (this.$values[id] !== undefined)
+      await binding.onValueChange(this.$values[id]);
     else if (this.$errors[id] !== undefined)
       binding.onValueError(this.$errors[id]);
 
@@ -819,15 +826,15 @@ class ShinyApp {
       }
     });
 
-    addMessageHandler("custom", (message: { [key: string]: unknown }) => {
+    addMessageHandler("custom", async (message: { [key: string]: unknown }) => {
       // For old-style custom messages - should deprecate and migrate to new
       // method
       const shinyOnCustomMessage = getShinyOnCustomMessage();
 
-      if (shinyOnCustomMessage) shinyOnCustomMessage(message);
+      if (shinyOnCustomMessage) await shinyOnCustomMessage(message);
 
       // Send messages.foo and messages.bar to appropriate handlers
-      this._sendMessagesToHandlers(
+      await this._sendMessagesToHandlers(
         message,
         customMessageHandlers,
         customMessageHandlerOrder

srcts/types/src/shiny/bind.d.ts

@@ -21,8 +21,8 @@ declare function bindInputs(shinyCtx: BindInputsCtx, scope?: BindScope): {
         };
     };
 };
-declare function _bindAll(shinyCtx: BindInputsCtx, scope: BindScope): ReturnType<typeof bindInputs>;
+declare function _bindAll(shinyCtx: BindInputsCtx, scope: BindScope): Promise<ReturnType<typeof bindInputs>>;
 declare function unbindAll(shinyCtx: BindInputsCtx, scope: BindScope, includeSelf?: boolean): void;
-declare function bindAll(shinyCtx: BindInputsCtx, scope: BindScope): void;
+declare function bindAll(shinyCtx: BindInputsCtx, scope: BindScope): Promise<void>;
 export { unbindAll, bindAll, _bindAll };
 export type { BindScope, BindInputsCtx };

srcts/types/src/shiny/init.d.ts

@@ -1,3 +1,3 @@
 import type { Shiny } from ".";
-declare function initShiny(windowShiny: Shiny): void;
+declare function initShiny(windowShiny: Shiny): Promise<void>;
 export { initShiny };

srcts/types/src/shiny/initedMethods.d.ts

@@ -11,10 +11,10 @@ declare function shinySetInputValue(name: string, value: unknown, opts?: {
 declare function shinyShinyApp(): ShinyApp;
 declare function setShinyUser(user: string): void;
 declare function shinyForgetLastInputValue(name: string): void;
-declare function shinyBindAll(scope: BindScope): void;
+declare function shinyBindAll(scope: BindScope): Promise<void>;
 declare function shinyUnbindAll(scope: BindScope, includeSelf?: boolean): void;
 declare function shinyInitializeInputs(scope: BindScope): void;
-declare function shinyAppBindOutput(id: string, binding: OutputBindingAdapter): void;
+declare function shinyAppBindOutput(id: string, binding: OutputBindingAdapter): Promise<void>;
 declare function shinyAppUnbindOutput(id: string, binding: OutputBindingAdapter): boolean;
 declare function getShinyOnCustomMessage(): Handler | null;
 declare function getFileInputBinding(): FileInputBinding;

srcts/types/src/shiny/reconnectDialog.d.ts

@@ -1,3 +1,3 @@
-declare function showReconnectDialog(delay: number): void;
+declare function showReconnectDialog(delay: number): Promise<void>;
 declare function hideReconnectDialog(): void;
 export { showReconnectDialog, hideReconnectDialog };

srcts/types/src/shiny/render.d.ts

@@ -1,14 +1,14 @@
 import type { BindScope } from "./bind";
 import { renderHtml as singletonsRenderHtml } from "./singletons";
 import type { WherePosition } from "./singletons";
+declare function renderContent(el: BindScope, content: string | {
+    html: string;
+    deps?: HtmlDep[];
+} | null, where?: WherePosition): Promise<void>;
 declare function renderContentAsync(el: BindScope, content: string | {
     html: string;
     deps?: HtmlDep[];
 } | null, where?: WherePosition): Promise<void>;
-declare function renderContent(el: BindScope, content: string | {
-    html: string;
-    deps?: HtmlDep[];
-} | null, where?: WherePosition): void;
 declare function renderHtmlAsync(html: string, el: BindScope, dependencies: HtmlDep[], where?: WherePosition): Promise<ReturnType<typeof singletonsRenderHtml>>;
 declare function renderHtml(html: string, el: BindScope, dependencies: HtmlDep[], where?: WherePosition): ReturnType<typeof singletonsRenderHtml>;
 declare function renderDependenciesAsync(dependencies: HtmlDep[] | null): Promise<void>;

srcts/types/src/shiny/shinyapp.d.ts

@@ -67,7 +67,7 @@ declare class ShinyApp {
     $sendMsg(msg: MessageValue): void;
     receiveError(name: string, error: ErrorsMessageValue): void;
     receiveOutput<T>(name: string, value: T): Promise<T | undefined>;
-    bindOutput(id: string, binding: OutputBindingAdapter): OutputBindingAdapter;
+    bindOutput(id: string, binding: OutputBindingAdapter): Promise<OutputBindingAdapter>;
     unbindOutput(id: string, binding: OutputBindingAdapter): boolean;
     private _narrowScopeComponent;
     private _narrowScope;