Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt another bind when registering a binding outside a renderHtml() context #3638

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
797 changes: 453 additions & 344 deletions inst/www/shared/shiny.js

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions inst/www/shared/shiny.js.map

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion inst/www/shared/shiny.min.js

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions inst/www/shared/shiny.min.js.map

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions srcts/src/bindings/registry.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { mergeSort } from "../utils";
import { Callbacks } from "../utils/callbacks";

interface BindingBase {
name: string;
Expand All @@ -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 };
Expand All @@ -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 {
Expand Down
15 changes: 14 additions & 1 deletion srcts/src/shiny/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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)
cpsievert marked this conversation as resolved.
Show resolved Hide resolved
const debouncedBindAll = debounce(0, () => windowShiny.bindAll!(document.documentElement))

const maybeBindOnRegister = () => {
if (!renderHtml.isExecuting()) debouncedBindAll();
};

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(
Expand Down
23 changes: 19 additions & 4 deletions srcts/src/shiny/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,13 @@ async function renderHtmlAsync(
dependencies: HtmlDep[],
where: WherePosition = "replace"
): Promise<ReturnType<typeof singletonsRenderHtml>> {
await renderDependenciesAsync(dependencies);
return singletonsRenderHtml(html, el, where);
renderHtml._renderCount++;
try {
await renderDependenciesAsync(dependencies);
return singletonsRenderHtml(html, el, where);
} finally {
renderHtml._renderCount--;
}
}

// Render HTML in a DOM element, inserting singletons into head as needed
Expand All @@ -144,10 +149,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;
};

// =============================================================================
// renderDependencies
// =============================================================================
Expand Down
45 changes: 45 additions & 0 deletions srcts/src/utils/callbacks.ts
Original file line number Diff line number Diff line change
@@ -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 };
3 changes: 3 additions & 0 deletions srcts/types/src/bindings/registry.d.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Callbacks } from "../utils/callbacks";
interface BindingBase {
name: string;
}
Expand All @@ -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>>;
Expand Down
4 changes: 4 additions & 0 deletions srcts/types/src/shiny/render.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ declare function renderContent(el: BindScope, content: string | {
} | 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 namespace renderHtml {
var _renderCount: number;
var isExecuting: () => boolean;
}
declare function renderDependenciesAsync(dependencies: HtmlDep[] | null): Promise<void>;
declare function renderDependencies(dependencies: HtmlDep[] | null): void;
declare type HtmlDepVersion = string;
Expand Down
16 changes: 16 additions & 0 deletions srcts/types/src/utils/callbacks.d.ts
Original file line number Diff line number Diff line change
@@ -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 };