Insert <head> content after script tags

yarn build (GitHub Actions)
Load script tags the normal way instead of with jQuery's synchronouse XHR
2026-01-12 08:27:56 -05:00 · 2022-07-07 22:01:01 -05:00 · 2022-07-07 20:01:30 +00:00 · 2022-07-07 14:54:29 -05:00 · 2022-07-07 13:37:37 -05:00 · 2022-07-07 13:36:21 -05:00
16 changed files with 1938 additions and 901 deletions
--- a/NEWS.md
+++ b/NEWS.md
@@ -23,6 +23,8 @@ shiny development

 ### Bug fixes

+* Closed #3657: `throttle.ts` and the `Throttler` typescript objects it provides now function as intended.
+
 * Closed tidyverse/dplyr#5552: Compatibility of dplyr 1.0 (and rlang chained errors in general) with `req()`, `validate()`, and friends.

 * Closed #1545: `insertUI()` now executes `<script>` tags. (#3630)
@@ -43,7 +45,9 @@ shiny development

 * Closed rstudio/shinytest2#222: When restoring a context (i.e., bookmarking) from a URL, Shiny now better handles a trailing `=` after `_inputs_` and `_values_`. (#3648)

-* Closed #3581: Errors in throttled/debounced reactive expressions cause session to exit. (#3624)
+* Closed #3581: Errors in throttled/debounced reactive expressions no longer cause the session to exit. (#3624)
+
+* Closed #3250:`{rlang}`/`{tidyeval}` conditions (i.e., warnings and errors) are no longer filtered from stack traces. (#3602)


 shiny 1.7.1
--- a/R/conditions.R
+++ b/R/conditions.R
@@ -421,8 +421,17 @@ pruneStackTrace <- function(parents) {
  # Loop over the parent indices. Anything that is not parented by current_node
  # (a.k.a. last-known-good node), or is a dupe, can be discarded. Anything that
  # is kept becomes the new current_node.
+  #
+  # jcheng 2022-03-18: Two more reasons a node can be kept:
+  #   1. parent is 0
+  #   2. parent is i
+  # Not sure why either of these situations happen, but they're common when
+  # interacting with rlang/dplyr errors. See issue rstudio/shiny#3250 for repro
+  # cases.
  include <- vapply(seq_along(parents), function(i) {
-    if (!is_dupe[[i]] && parents[[i]] == current_node) {
+    if ((!is_dupe[[i]] && parents[[i]] == current_node) ||
+        parents[[i]] == 0 ||
+        parents[[i]] == i) {
      current_node <<- i
      TRUE
    } else {
--- a/inst/www/shared/shiny.js
+++ b/inst/www/shared/shiny.js
--- a/inst/www/shared/shiny.js.map
+++ b/inst/www/shared/shiny.js.map
--- a/inst/www/shared/shiny.min.js
+++ b/inst/www/shared/shiny.min.js
--- a/inst/www/shared/shiny.min.js.map
+++ b/inst/www/shared/shiny.min.js.map
--- a/srcts/src/bindings/registry.ts
+++ b/srcts/src/bindings/registry.ts
@@ -1,4 +1,5 @@
 import { mergeSort } from "../utils";
+import { Callbacks } from "../utils/callbacks";

 interface BindingBase {
  name: string;
@@ -14,6 +15,7 @@ class BindingRegistry<Binding extends BindingBase> {
  name: string;
  bindings: Array<BindingObj<Binding>> = [];
  bindingNames: { [key: string]: BindingObj<Binding> } = {};
+  registerCallbacks: Callbacks = new Callbacks();

  register(binding: Binding, bindingName: string, priority = 0): void {
    const bindingObj = { binding, priority };
@@ -23,6 +25,12 @@ class BindingRegistry<Binding extends BindingBase> {
      this.bindingNames[bindingName] = bindingObj;
      binding.name = bindingName;
    }
+
+    this.registerCallbacks.invoke();
+  }
+
+  onRegister(fn: () => void, once = true): void {
+    this.registerCallbacks.register(fn, once);
  }

  setPriority(bindingName: string, priority: number): void {
--- a/srcts/src/shiny/init.ts
+++ b/srcts/src/shiny/init.ts
@@ -21,7 +21,7 @@ import {
 import { bindAll, unbindAll, _bindAll } from "./bind";
 import type { BindInputsCtx, BindScope } from "./bind";
 import { setShinyObj } from "./initedMethods";
-import { registerDependency } from "./render";
+import { registerDependency, renderHtml } from "./render";
 import { sendImageSizeFns } from "./sendImageSize";
 import { ShinyApp } from "./shinyapp";
 import { registerNames as singletonsRegisterNames } from "./singletons";
@@ -150,6 +150,19 @@ function initShiny(windowShiny: Shiny): void {
    (x) => x.value
  );

+  // When future bindings are registered via dynamic UI, check to see if renderHtml()
+  // is currently executing. If it's not, it's likely that the binding registration
+  // is occurring a tick after renderHtml()/renderContent(), in which case we need
+  // to make sure the new bindings get a chance to bind to the DOM. (#3635)
+  const maybeBindOnRegister = debounce(0, () => {
+    if (!renderHtml.isExecuting()) {
+      windowShiny.bindAll(document.documentElement);
+    }
+  });
+
+  inputBindings.onRegister(maybeBindOnRegister, false);
+  outputBindings.onRegister(maybeBindOnRegister, false);
+
  // The server needs to know the size of each image and plot output element,
  // in case it is auto-sizing
  $(".shiny-image-output, .shiny-plot-output, .shiny-report-size").each(
--- a/srcts/src/shiny/render.ts
+++ b/srcts/src/shiny/render.ts
@@ -72,10 +72,20 @@ function renderHtml(
  dependencies: HtmlDep[],
  where: WherePosition = "replace"
 ): ReturnType<typeof singletonsRenderHtml> {
-  renderDependencies(dependencies);
-  return singletonsRenderHtml(html, el, where);
+  renderHtml._renderCount++;
+  try {
+    renderDependencies(dependencies);
+    return singletonsRenderHtml(html, el, where);
+  } finally {
+    renderHtml._renderCount--;
+  }
 }

+renderHtml._renderCount = 0;
+renderHtml.isExecuting = function () {
+  return renderHtml._renderCount > 0;
+};
+
 type HtmlDepVersion = string;

 type MetaItem = {
@@ -199,6 +209,9 @@ function renderDependency(dep_: HtmlDep) {
    $head.append(stylesheetLinks);
  }

+  const scriptPromises: Array<Promise<any>> = [];
+  const scriptElements: HTMLScriptElement[] = [];
+
  dep.script.forEach((x) => {
    const script = document.createElement("script");

@@ -210,9 +223,23 @@ function renderDependency(dep_: HtmlDep) {
      script.setAttribute(attr, val ? val : "");
    });

-    $head.append(script);
+    const p = new Promise((resolve) => {
+      // eslint-disable-next-line @typescript-eslint/no-unused-vars
+      script.onload = (e: Event) => {
+        resolve(null);
+      };
+    });
+
+    scriptPromises.push(p);
+    scriptElements.push(script);
  });

+  // Append the script elements all at once, so that we're sure they'll load in
+  // order. (We didn't append them individually in the `forEach()` above,
+  // because we're not sure that the browser will load them in order if done
+  // that way.)
+  document.head.append(...scriptElements);
+
  dep.attachment.forEach((x) => {
    const link = $("<link rel='attachment'>")
      .attr("id", dep.name + "-" + x.key + "-attachment")
@@ -221,12 +248,22 @@ function renderDependency(dep_: HtmlDep) {
    $head.append(link);
  });

-  if (dep.head) {
-    const $newHead = $("<head></head>");
+  Promise.allSettled(scriptPromises).then(() => {
+    // After the scripts are all loaded, insert any head content. This may
+    // contain <script> tags with inline content, which we want to execute after
+    // the script elements above, because the code here may depend on them.
+    if (dep.head) {
+      const $newHead = $("<head></head>");
+
+      $newHead.html(dep.head);
+      $head.append($newHead.children());
+    }
+
+    // Bind all
+    shinyInitializeInputs(document.body);
+    shinyBindAll(document.body);
+  });

-    $newHead.html(dep.head);
-    $head.append($newHead.children());
-  }
  return true;
 }

--- a/srcts/src/time/throttle.ts
+++ b/srcts/src/time/throttle.ts
@@ -19,40 +19,67 @@ class Throttler<X extends AnyVoidFunction> implements InputRatePolicy<X> {
    this.args = null;
  }

+  // If no timer is currently running, immediately call the function and set the
+  // timer; if a timer is running out, just queue up the args for the call when
+  // the timer runs out. Later calls during the same timeout will overwrite
+  // earlier ones.
  normalCall(...args: Parameters<X>): void {
+    // This will be an empty array (not null) if called without arguments, and
+    // `[null]` if called with `null`.
    this.args = args;
+
+    // Only invoke immediately if there isn't a timer running.
    if (this.timerId === null) {
      this.$invoke();
-      this.timerId = setTimeout(() => {
-        // IE8 doesn't reliably clear timeout, so this additional
-        // check is needed
-        if (this.timerId === null) return;
-        this.$clearTimer();
-        if (args.length > 0) this.normalCall(...args);
-      }, this.delayMs);
    }
  }
+
+  // Reset the timer if active and call immediately
  immediateCall(...args: Parameters<X>): void {
    this.$clearTimer();
    this.args = args;
    this.$invoke();
  }
+
+  // Is there a call waiting to send?
  isPending(): boolean {
-    return this.timerId !== null;
+    return this.args !== null;
  }
+
  $clearTimer(): void {
    if (this.timerId !== null) {
      clearTimeout(this.timerId);
      this.timerId = null;
    }
  }
+
+  // Invoke the throttled function with the currently-stored args and start the
+  // timer.
  $invoke(): void {
-    if (this.args && this.args.length > 0) {
-      this.func.apply(this.target, this.args);
-    } else {
-      this.func.apply(this.target);
+    if (this.args === null) {
+      // Shouldn't get here, because $invoke should only be called right after
+      // setting this.args. But just in case.
+      return;
    }
+
+    this.func.apply(this.target, this.args);
+
+    // Clear the stored args. This is used to track if a call is pending.
    this.args = null;
+
+    // Set this.timerId to a newly-created timer, which will invoke a call with
+    // the most recently called args (if any) when it expires.
+    this.timerId = setTimeout(() => {
+      // IE8 doesn't reliably clear timeout, so this additional check is needed
+      if (this.timerId === null) return;
+
+      this.$clearTimer();
+      // Do we have a call queued up?
+      if (this.isPending()) {
+        // If so, invoke the call with queued args and reset timer.
+        this.$invoke();
+      }
+    }, this.delayMs);
  }
 }

--- a/srcts/src/utils/callbacks.ts
+++ b/srcts/src/utils/callbacks.ts
@@ -0,0 +1,45 @@
+type Cb = {
+  once: boolean;
+  fn: () => void;
+};
+
+type Cbs = {
+  [key: string]: Cb;
+};
+
+class Callbacks {
+  callbacks: Cbs = {};
+  id = 0;
+
+  register(fn: () => void, once = true): () => void {
+    this.id += 1;
+    const id = this.id;
+
+    this.callbacks[id] = { fn, once };
+    return () => {
+      delete this.callbacks[id];
+    };
+  }
+
+  invoke(): void {
+    for (const id in this.callbacks) {
+      const cb = this.callbacks[id];
+
+      try {
+        cb.fn();
+      } finally {
+        if (cb.once) delete this.callbacks[id];
+      }
+    }
+  }
+
+  clear(): void {
+    this.callbacks = {};
+  }
+
+  count(): number {
+    return Object.keys(this.callbacks).length;
+  }
+}
+
+export { Callbacks };
--- a/srcts/types/src/bindings/registry.d.ts
+++ b/srcts/types/src/bindings/registry.d.ts
@@ -1,3 +1,4 @@
+import { Callbacks } from "../utils/callbacks";
 interface BindingBase {
    name: string;
 }
@@ -12,7 +13,9 @@ declare class BindingRegistry<Binding extends BindingBase> {
    bindingNames: {
        [key: string]: BindingObj<Binding>;
    };
+    registerCallbacks: Callbacks;
    register(binding: Binding, bindingName: string, priority?: number): void;
+    onRegister(fn: () => void, once?: boolean): void;
    setPriority(bindingName: string, priority: number): void;
    getPriority(bindingName: string): number | false;
    getBindings(): Array<BindingObj<Binding>>;
--- a/srcts/types/src/shiny/render.d.ts
+++ b/srcts/types/src/shiny/render.d.ts
@@ -7,6 +7,10 @@ declare function renderContent(el: BindScope, content: string | {
    deps?: HtmlDep[];
 } | null, where?: WherePosition): void;
 declare function renderHtml(html: string, el: BindScope, dependencies: HtmlDep[], where?: WherePosition): ReturnType<typeof singletonsRenderHtml>;
+declare namespace renderHtml {
+    var _renderCount: number;
+    var isExecuting: () => boolean;
+}
 declare type HtmlDepVersion = string;
 declare type MetaItem = {
    name: string;
--- a/srcts/types/src/utils/callbacks.d.ts
+++ b/srcts/types/src/utils/callbacks.d.ts
@@ -0,0 +1,16 @@
+declare type Cb = {
+    once: boolean;
+    fn: () => void;
+};
+declare type Cbs = {
+    [key: string]: Cb;
+};
+declare class Callbacks {
+    callbacks: Cbs;
+    id: number;
+    register(fn: () => void, once?: boolean): () => void;
+    invoke(): void;
+    clear(): void;
+    count(): number;
+}
+export { Callbacks };
--- a/tests/testthat/test-stacks-pruning.R
+++ b/tests/testthat/test-stacks-pruning.R
@@ -1,30 +1,33 @@
-capture <- function() {
-  list(
-    calls = sys.calls(),
-    parents = sys.parents()
+foo <- function() {
+  capture <- function() {
+    list(
+      calls = sys.calls(),
+      parents = sys.parents()
+    )
+  }
+
+  capture_1 <- function() {
+    capture()
+  }
+
+  capture_2 <- function() {
+    capture_1()
+  }
+
+  do.call(
+    identity,
+    list(
+      identity(capture_2())
+    )
  )
 }
-
-capture_1 <- function() {
-  capture()
-}
-
-capture_2 <- function() {
-  capture_1()
-}
-
-res <- do.call(
-  identity,
-  list(
-    identity(capture_2())
-  )
-)
-res$calls <- tail(res$calls, 5)
-res$parents <- tail(res$parents - (length(res$parents) - 5), 5)
+res <- foo()
+res$calls <- tail(res$calls, 6)
+res$parents <- tail(res$parents - (length(res$parents) - 6), 6)

 describe("stack pruning", {
  it("passes basic example", {
-    expect_equal(pruneStackTrace(res$parents), c(F, F, T, T, T))
-    expect_equal(lapply(list(res$parents), pruneStackTrace), list(c(F, F, T, T, T)))
+    expect_equal(pruneStackTrace(res$parents), c(T, F, F, T, T, T))
+    expect_equal(lapply(list(res$parents), pruneStackTrace), list(c(T, F, F, T, T, T)))
  })
 })
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -1,12 +1,13 @@
 {
  "declaration": true,
  "compilerOptions": {
-    "target": "ES5",
+    "target": "es2020",
    "isolatedModules": true,
    "esModuleInterop": true,
    "declaration": true,
    "declarationDir": "./srcts/types",
    "emitDeclarationOnly": true,
+    "moduleResolution": "node",
    // Can not use `types: []` to disable injecting NodeJS types. More types are
    // needed than just the DOM's `window.setTimeout`
    // "types": [],
Author	SHA1	Message	Date
Winston Chang	2e934797c9	Insert <head> content after script tags	2022-07-07 22:01:01 -05:00
wch	581ace76e4	`yarn build` (GitHub Actions)	2022-07-07 20:01:30 +00:00
Winston Chang	e43609b60a	Load script tags the normal way instead of with jQuery's synchronouse XHR	2022-07-07 14:54:29 -05:00
Carson	d0bf86e5e2	Add a clear() method to Callbacks	2022-07-07 13:37:37 -05:00
Carson	b023350b90	Introduce an onRegister() method on BindingRegistry to help solve the problem with sharing state	2022-07-07 13:36:21 -05:00
Carson	7bccfeb774	Close #3635 : attempt another bind when registering a binding outside a renderHtml() context	2022-07-07 13:35:07 -05:00
Winston Chang	54e5a6b43c	Merge branch 'dvg-p4-fix-throttle'	2022-07-05 20:03:22 -05:00
Winston Chang	9653cc2893	Rebuild shiny.js	2022-07-05 20:01:22 -05:00
Winston Chang	47dc5b4116	Code and comment cleanup	2022-07-05 19:37:44 -05:00
dvg-p4	9db9ef527a	Fixed check for isPending and rebuilt javascript	2022-07-04 10:21:22 -04:00
dvg-p4	9285a1f7fc	Update srcts/src/time/throttle.ts Based on suggestion Co-authored-by: Winston Chang <winston@stdout.org>	2022-07-01 19:02:26 -04:00
dvg-p4	d22eb1524a	Updated NEWS.md	2022-07-01 17:15:09 -04:00
dvg-p4	5e3971c776	Fixed major bug in throttle.ts	2022-07-01 16:58:41 -04:00
Joe Cheng	ff5ef52dd5	Fix #3250 (#3602 ) * Fix #3250 pruneStackTrace was interacting badly with dplyr errors. I'm still not sure what causes these new cases, but the new behavior seems to be much better, with no downside that I can think of. * Fix existing unit tests * Update news Co-authored-by: Carson Sievert <cpsievert1@gmail.com>	2022-06-27 12:05:28 -05:00