Merge branch 'main' into chat-shiny-bind

Author: cpsievert

CHANGELOG.md

@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changes
 
+* Express mode's `app_opts()` requires all arguments to be keyword-only. If you are using positional arguments, you will need to update your code. (#1895)
+
 * The `.get_latest_stream_result()` method on `ui.MarkdownStream()` was deprecated in favor of the new `.latest_stream` property. Call `.result()` on the property to get the latest result, `.status` to check the status, and `.cancel()` to cancel the stream.
 
 ### Bug fixes

js/build.ts

@@ -1,17 +1,59 @@
-import { BuildOptions, build } from "esbuild";
+import { BuildOptions, build, type Metafile } from "esbuild";
 import { sassPlugin } from "esbuild-sass-plugin";
 import * as fs from "node:fs/promises";
 
 let minify = true;
+let metafile = true;
 process.argv.forEach((val, index) => {
   if (val === "--minify=false") {
     console.log("Disabling minification");
     minify = false;
   }
+  if (val === "--metafile=false") {
+    console.log("Disabling metafile generation");
+    metafile = false;
+  }
 });
 
 const outDir = "../shiny/www/py-shiny";
 
+const allEsbuildMetadata: Array<Metafile> = [];
+
+function mergeMetadatas(metadatas: Array<Metafile>): Metafile {
+  // Merge all the metafile objects together
+  const mergedMetadata: Metafile = {
+    inputs: {},
+    outputs: {},
+  };
+
+  metadatas.forEach((metafile) => {
+    Object.entries(metafile.inputs).forEach(([key, value]) => {
+      if (
+        mergedMetadata.inputs[key] &&
+        JSON.stringify(mergedMetadata.inputs[key]) !== JSON.stringify(value)
+      ) {
+        // It's very possible for multiple MetaFile objects to refer to the same input.
+        // But if that input has different values in the different Metafile objects,
+        // that could cause inaccuracies when we merge them. I think it's possible they
+        // could have different values if tree-shaking is enabled -- this will detect
+        // those cases and warn the user, and if it happens we can figure out how to
+        // handle it.
+        console.error(
+          `Different values found for key in metadata: ${key}. Overwriting.`
+        );
+      }
+      mergedMetadata.inputs[key] = value;
+    });
+    Object.entries(metafile.outputs).forEach(([key, value]) => {
+      if (mergedMetadata.outputs[key]) {
+        console.error(`Duplicate key found in metadata: ${key}. Overwriting.`);
+      }
+      mergedMetadata.outputs[key] = value;
+    });
+  });
+
+  return mergedMetadata;
+}
 async function bundle_helper(
   options: BuildOptions
 ): Promise<ReturnType<typeof build> | undefined> {
@@ -20,8 +62,10 @@ async function bundle_helper(
       format: "esm",
       bundle: true,
       minify: minify,
-      sourcemap: true,
-      metafile: false,
+      // No need to clean up old source maps, as `minify==false` only during `npm run watch-fast`
+      // GHA will run `npm run build` which will minify
+      sourcemap: minify,
+      metafile: metafile,
       outdir: outDir,
       ...options,
     });
@@ -34,15 +78,10 @@ async function bundle_helper(
       }
     );
 
-    if (options.metafile) {
-      // Save metafile
-      const data_frame_results = result;
-      await fs.writeFile(
-        "esbuild-metadata.json",
-        JSON.stringify(data_frame_results.metafile)
-      );
-      console.log("Metadata file written to esbuild-metadata.json");
+    if (result.metafile) {
+      allEsbuildMetadata.push(result.metafile);
     }
+
     return result;
   } catch (error) {
     console.error("Build failed:", error);
@@ -53,57 +92,49 @@ const opts: Array<BuildOptions> = [
   {
     entryPoints: { "data-frame/data-frame": "data-frame/index.tsx" },
     plugins: [sassPlugin({ type: "css-text", sourceMap: false })],
-    metafile: true,
   },
   {
     entryPoints: {
       "text-area/textarea-autoresize": "text-area/textarea-autoresize.ts",
     },
-    minify: false,
-    sourcemap: false,
   },
   {
     entryPoints: {
       "page-output/page-output": "page-output/page-output.ts",
     },
-    minify: false,
-    sourcemap: false,
   },
   {
     entryPoints: { "spin/spin": "spin/spin.scss" },
     plugins: [sassPlugin({ type: "css", sourceMap: false })],
-    metafile: true,
   },
   {
     entryPoints: {
       "markdown-stream/markdown-stream": "markdown-stream/markdown-stream.ts",
     },
-    minify: true,
-    sourcemap: true,
   },
   {
     entryPoints: {
       "markdown-stream/markdown-stream": "markdown-stream/markdown-stream.scss",
     },
     plugins: [sassPlugin({ type: "css", sourceMap: false })],
-    metafile: true,
   },
   {
     entryPoints: {
       "chat/chat": "chat/chat.ts",
     },
-    minify: true,
-    sourcemap: true,
   },
   {
     entryPoints: { "chat/chat": "chat/chat.scss" },
     plugins: [sassPlugin({ type: "css", sourceMap: false })],
-    metafile: true,
   },
 ];
 
-// Run function to avoid top level await
-async function main(): Promise<void> {
+(async () => {
   await Promise.all(opts.map(bundle_helper));
-}
-main();
+
+  if (metafile) {
+    const mergedMetadata = mergeMetadatas(allEsbuildMetadata);
+    await fs.writeFile("esbuild-metadata.json", JSON.stringify(mergedMetadata));
+    console.log("Metadata file written to esbuild-metadata.json");
+  }
+})();

js/chat/chat.scss

@@ -32,7 +32,7 @@ shiny-chat-container {
     text-underline-offset: 4px;
     text-decoration-thickness: 2px;
 
-    padding: 2px;
+    padding-inline: 2px;
 
     &:hover {
       text-decoration-style: solid;

js/chat/chat.ts

@@ -231,7 +231,7 @@ class ChatInput extends LightElement {
     if (this.valueIsEmpty) return;
     if (this.disabled) return;
 
-    Shiny.setInputValue!(this.id, this.value, { priority: "event" });
+    window.Shiny.setInputValue!(this.id, this.value, { priority: "event" });
 
     // Emit event so parent element knows to insert the message
     const sentEvent = new CustomEvent("shiny-chat-input-sent", {
@@ -534,33 +534,30 @@ customElements.define(CHAT_MESSAGES_TAG, ChatMessages);
 customElements.define(CHAT_INPUT_TAG, ChatInput);
 customElements.define(CHAT_CONTAINER_TAG, ChatContainer);
 
-$(function () {
-  Shiny.addCustomMessageHandler(
-    "shinyChatMessage",
-    async function (message: ShinyChatMessage) {
-
-      if (message.obj?.html_deps) {
-        await renderDependencies(message.obj.html_deps);
-      }
+window.Shiny.addCustomMessageHandler(
+  "shinyChatMessage",
+  async function (message: ShinyChatMessage) {
+    if (message.obj?.html_deps) {
+      await renderDependencies(message.obj.html_deps);
+    }
 
-      const evt = new CustomEvent(message.handler, {
-        detail: message.obj,
-      });
+    const evt = new CustomEvent(message.handler, {
+      detail: message.obj,
+    });
 
-      const el = document.getElementById(message.id);
+    const el = document.getElementById(message.id);
 
-      if (!el) {
-        showShinyClientMessage({
-          status: "error",
-          message: `Unable to handle Chat() message since element with id
+    if (!el) {
+      showShinyClientMessage({
+        status: "error",
+        message: `Unable to handle Chat() message since element with id
           ${message.id} wasn't found. Do you need to call .ui() (Express) or need a
           chat_ui('${message.id}') in the UI (Core)?
         `,
-        });
-        return;
-      }
-
-      el.dispatchEvent(evt);
+      });
+      return;
     }
-  );
-});
+
+    el.dispatchEvent(evt);
+  }
+);

js/data-frame/index.tsx

@@ -527,7 +527,7 @@ const ShinyDataGrid: FC<ShinyDataGridProps<unknown>> = ({
   useEffect(() => {
     if (!htmlDeps) return;
     // Register the Shiny HtmlDependencies
-    Shiny.renderDependenciesAsync([...htmlDeps]);
+    window.Shiny.renderDependenciesAsync([...htmlDeps]);
   }, [htmlDeps]);
 
   useEffect(() => {
@@ -701,7 +701,7 @@ const ShinyDataGrid: FC<ShinyDataGridProps<unknown>> = ({
     } else {
       console.error("Unhandled row selection mode:", selectionModes);
     }
-    Shiny.setInputValue!(`${id}_cell_selection`, shinyValue);
+    window.Shiny.setInputValue!(`${id}_cell_selection`, shinyValue);
   }, [id, selection, selectionModes, table, table.getSortedRowModel]);
 
   useEffect(() => {
@@ -714,10 +714,10 @@ const ShinyDataGrid: FC<ShinyDataGridProps<unknown>> = ({
         desc: sortObj.desc,
       });
     });
-    Shiny.setInputValue!(`${id}_sort`, shinySort);
+    window.Shiny.setInputValue!(`${id}_sort`, shinySort);
 
     // Deprecated as of 2024-05-21
-    Shiny.setInputValue!(`${id}_column_sort`, shinySort);
+    window.Shiny.setInputValue!(`${id}_column_sort`, shinySort);
   }, [columns, id, sorting]);
   useEffect(() => {
     if (!id) return;
@@ -732,10 +732,10 @@ const ShinyDataGrid: FC<ShinyDataGridProps<unknown>> = ({
         value: filterObj.value as FilterValue,
       });
     });
-    Shiny.setInputValue!(`${id}_filter`, shinyFilter);
+    window.Shiny.setInputValue!(`${id}_filter`, shinyFilter);
 
     // Deprecated as of 2024-05-21
-    Shiny.setInputValue!(`${id}_column_filter`, shinyFilter);
+    window.Shiny.setInputValue!(`${id}_column_filter`, shinyFilter);
   }, [id, columnFilters, columns]);
   useEffect(() => {
     if (!id) return;
@@ -744,10 +744,10 @@ const ShinyDataGrid: FC<ShinyDataGridProps<unknown>> = ({
       // Already prefiltered rows!
       .getSortedRowModel()
       .rows.map((row) => row.index);
-    Shiny.setInputValue!(`${id}_data_view_rows`, shinyRows);
+    window.Shiny.setInputValue!(`${id}_data_view_rows`, shinyRows);
 
     // Legacy value as of 2024-05-13
-    Shiny.setInputValue!(`${id}_data_view_indices`, shinyRows);
+    window.Shiny.setInputValue!(`${id}_data_view_indices`, shinyRows);
   }, [
     id,
     table,
@@ -773,7 +773,7 @@ const ShinyDataGrid: FC<ShinyDataGridProps<unknown>> = ({
         .filter((x): x is number => x !== null)
         .sort();
     }
-    Shiny.setInputValue!(`${id}_selected_rows`, shinyValue);
+    window.Shiny.setInputValue!(`${id}_selected_rows`, shinyValue);
   }, [id, selection, selectionModes, table]);
 
   // ### End row selection ############################################################
@@ -1083,7 +1083,7 @@ function useVirtualizerMeasureWorkaround(
   return measureElementWithRetry;
 }
 
-class ShinyDataFrameOutputBinding extends Shiny.OutputBinding {
+class ShinyDataFrameOutputBinding extends window.Shiny.OutputBinding {
   find(scope: HTMLElement | JQuery<HTMLElement>): JQuery<HTMLElement> {
     return $(scope).find("shiny-data-frame");
   }
@@ -1103,7 +1103,7 @@ class ShinyDataFrameOutputBinding extends Shiny.OutputBinding {
     el.clearError();
   }
 }
-Shiny.outputBindings.register(
+window.Shiny.outputBindings.register(
   new ShinyDataFrameOutputBinding(),
   "shinyDataFrame"
 );
@@ -1214,12 +1214,13 @@ customElements.define("shiny-data-frame", ShinyDataFrameOutput);
 // react listener.
 // It would be better to have something similar to session.send_input_message
 // for updating outputs, but that requires changes to ShinyJS.
-$(function () {
-  Shiny.addCustomMessageHandler("shinyDataFrameMessage", function (message) {
+window.Shiny.addCustomMessageHandler(
+  "shinyDataFrameMessage",
+  function (message) {
     const evt = new CustomEvent(message.handler, {
       detail: message.obj,
     });
     const el = document.getElementById(message.id);
     el?.dispatchEvent(evt);
-  });
-});
+  }
+);

js/markdown-stream/markdown-stream.ts

@@ -335,8 +335,9 @@ async function handleMessage(
   }
 }
 
-$(function () {
-  Shiny.addCustomMessageHandler("shinyMarkdownStreamMessage", handleMessage);
-});
+window.Shiny.addCustomMessageHandler(
+  "shinyMarkdownStreamMessage",
+  handleMessage
+);
 
 export { MarkdownElement, contentToHTML };