From 5aaa2497e6c8d90e8bef582ae808bce05cbad8d2 Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Mon, 25 Aug 2025 11:50:05 -0400 Subject: [PATCH 01/20] including notes --- shiny/ui/_include_helpers.py | 25 +++++++++++++++++-- .../shiny/bugs/2061-css-js-inclusion/app.py | 22 ++++++++++++++++ .../2061-css-js-inclusion/www/customjs.js | 1 + .../bugs/2061-css-js-inclusion/www/style.css | 19 ++++++++++++++ 4 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py create mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/www/customjs.js create mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/www/style.css diff --git a/shiny/ui/_include_helpers.py b/shiny/ui/_include_helpers.py index 3c0c9409a..826482c9b 100644 --- a/shiny/ui/_include_helpers.py +++ b/shiny/ui/_include_helpers.py @@ -89,6 +89,10 @@ def include_js( if method == "inline": return tags.script(read_utf8(file_path), **kwargs) + # QUESTION: Do we have access to the session or user at this point? Can write based on that and then give them full perms? + # Where does cleanup happen?! Seems like maybe that needs to be included before we make new folders? Check the date? Tell if active + # session using them?! + include_files = method == "link_files" path_dest, hash = maybe_copy_files(file_path, include_files) @@ -217,21 +221,35 @@ def maybe_copy_files(path: Path | str, include_files: bool) -> tuple[str, str]: # To avoid unnecessary work when the same file is included multiple times, # use a directory scoped by a hash of the file. + # We need to make tempdir deterministic if we use the pythong TemporaryDirectory class so we can still tell if the hash has changed + # We want the temp dir unique to the process, hash should be the only thing that is dynamic based on the file contents. + # Cleanup done using __del__? + # Also make perms so that others can read it + # Double check clean-up based on process ending tmpdir = os.path.join(tempfile.gettempdir(), "shiny_include_files", hash) path_dest = os.path.join(tmpdir, os.path.basename(path)) + print("tmpdir, path_dest: ", tmpdir, path_dest) # Since the hash/tmpdir should represent all the files in the path's directory, # we can simply return here if os.path.exists(path_dest): + print("path exists: ", path_dest, hash) return path_dest, hash # Otherwise, make sure we have a clean slate + # We only hit this if tempdir/ exists but not the file (i.e. style.css) if os.path.exists(tmpdir): + print("clean the slate, tmpdir", tmpdir) shutil.rmtree(tmpdir) if include_files: + print("include files true") + # TODO: Switch this to tempfile.mkdtmp? This will scope perms to the userID + # QUESTION what is the user id? The system user? How does that work in Connect? shutil.copytree(os.path.dirname(path), tmpdir) else: + print("include files false") + # This should probably use the temp file syntax os.makedirs(tmpdir, exist_ok=True) shutil.copy(path, path_dest) @@ -240,11 +258,14 @@ def maybe_copy_files(path: Path | str, include_files: bool) -> tuple[str, str]: def get_hash(path: Path | str, include_files: bool) -> str: if include_files: - key = get_file_key(path) - else: dir = os.path.dirname(path) + # This is recursively listing everything in the dir and making that part of the hash even though we're not pulling it over + # Ex. we make a hash, we copy over one file, then we jsut don't keep going files = glob.iglob(os.path.join(dir, "**"), recursive=True) key = "\n".join([get_file_key(x) for x in files]) + else: + key = get_file_key(path) + return hash_deterministic(key) diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py new file mode 100644 index 000000000..438af56dd --- /dev/null +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py @@ -0,0 +1,22 @@ +from shiny import App, ui + +# Define the UI +app_ui = ui.page_fluid( + ui.include_css("./www/style.css"), # Reference the external CSS file + ui.include_js("./www/customjs.js"), + ui.h1("Simple Shiny App with External CSS"), + ui.div( + ui.p("This is a simple Shiny app that demonstrates ui.include_css()"), + ui.p("The styling comes from an external CSS file!"), + class_="content", + ), +) + + +# Define the server +def server(input, output, session): + pass + + +# Create and run the app +app = App(app_ui, server) diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/www/customjs.js b/tests/playwright/shiny/bugs/2061-css-js-inclusion/www/customjs.js new file mode 100644 index 000000000..dbb1b36fa --- /dev/null +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/www/customjs.js @@ -0,0 +1 @@ +console.log("heyo") \ No newline at end of file diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/www/style.css b/tests/playwright/shiny/bugs/2061-css-js-inclusion/www/style.css new file mode 100644 index 000000000..e86131d86 --- /dev/null +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/www/style.css @@ -0,0 +1,19 @@ + +body { + background-color: #f0f8ff; + font-family: Arial, sans-serif; +} + +h1 { + color: #4682b4; + border-bottom: 2px solid #4682b4; + padding-bottom: 10px; +} + +.content { + margin: 20px; + padding: 15px; + background-color: white; + border-radius: 5px; + box-shadow: 0 0 10px rgba(0, 0, 0, 0.1); +} From cc15245fc397f92ce84850ce99dac2a534032564 Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Mon, 25 Aug 2025 12:23:36 -0400 Subject: [PATCH 02/20] Removing notes to self --- shiny/ui/_include_helpers.py | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/shiny/ui/_include_helpers.py b/shiny/ui/_include_helpers.py index 826482c9b..0a9be9075 100644 --- a/shiny/ui/_include_helpers.py +++ b/shiny/ui/_include_helpers.py @@ -89,10 +89,6 @@ def include_js( if method == "inline": return tags.script(read_utf8(file_path), **kwargs) - # QUESTION: Do we have access to the session or user at this point? Can write based on that and then give them full perms? - # Where does cleanup happen?! Seems like maybe that needs to be included before we make new folders? Check the date? Tell if active - # session using them?! - include_files = method == "link_files" path_dest, hash = maybe_copy_files(file_path, include_files) @@ -219,13 +215,6 @@ def create_include_dependency( def maybe_copy_files(path: Path | str, include_files: bool) -> tuple[str, str]: hash = get_hash(path, include_files) - # To avoid unnecessary work when the same file is included multiple times, - # use a directory scoped by a hash of the file. - # We need to make tempdir deterministic if we use the pythong TemporaryDirectory class so we can still tell if the hash has changed - # We want the temp dir unique to the process, hash should be the only thing that is dynamic based on the file contents. - # Cleanup done using __del__? - # Also make perms so that others can read it - # Double check clean-up based on process ending tmpdir = os.path.join(tempfile.gettempdir(), "shiny_include_files", hash) path_dest = os.path.join(tmpdir, os.path.basename(path)) print("tmpdir, path_dest: ", tmpdir, path_dest) @@ -233,23 +222,15 @@ def maybe_copy_files(path: Path | str, include_files: bool) -> tuple[str, str]: # Since the hash/tmpdir should represent all the files in the path's directory, # we can simply return here if os.path.exists(path_dest): - print("path exists: ", path_dest, hash) return path_dest, hash # Otherwise, make sure we have a clean slate - # We only hit this if tempdir/ exists but not the file (i.e. style.css) if os.path.exists(tmpdir): - print("clean the slate, tmpdir", tmpdir) shutil.rmtree(tmpdir) if include_files: - print("include files true") - # TODO: Switch this to tempfile.mkdtmp? This will scope perms to the userID - # QUESTION what is the user id? The system user? How does that work in Connect? shutil.copytree(os.path.dirname(path), tmpdir) else: - print("include files false") - # This should probably use the temp file syntax os.makedirs(tmpdir, exist_ok=True) shutil.copy(path, path_dest) @@ -259,8 +240,6 @@ def maybe_copy_files(path: Path | str, include_files: bool) -> tuple[str, str]: def get_hash(path: Path | str, include_files: bool) -> str: if include_files: dir = os.path.dirname(path) - # This is recursively listing everything in the dir and making that part of the hash even though we're not pulling it over - # Ex. we make a hash, we copy over one file, then we jsut don't keep going files = glob.iglob(os.path.join(dir, "**"), recursive=True) key = "\n".join([get_file_key(x) for x in files]) else: From e6b8260add9a2a25bf047996ed8cba7037910266 Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Mon, 25 Aug 2025 12:27:34 -0400 Subject: [PATCH 03/20] cleanup --- shiny/ui/_include_helpers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/shiny/ui/_include_helpers.py b/shiny/ui/_include_helpers.py index 0a9be9075..33840b94b 100644 --- a/shiny/ui/_include_helpers.py +++ b/shiny/ui/_include_helpers.py @@ -215,9 +215,10 @@ def create_include_dependency( def maybe_copy_files(path: Path | str, include_files: bool) -> tuple[str, str]: hash = get_hash(path, include_files) + # To avoid unnecessary work when the same file is included multiple times, + # use a directory scoped by a hash of the file. tmpdir = os.path.join(tempfile.gettempdir(), "shiny_include_files", hash) path_dest = os.path.join(tmpdir, os.path.basename(path)) - print("tmpdir, path_dest: ", tmpdir, path_dest) # Since the hash/tmpdir should represent all the files in the path's directory, # we can simply return here From f4c53fdcf62c36545a891c7684a3164a51f81235 Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Tue, 26 Aug 2025 13:15:10 -0400 Subject: [PATCH 04/20] With prints --- shiny/ui/_include_helpers.py | 32 ++++++++++++++++--- .../shiny/bugs/2061-css-js-inclusion/app.py | 4 ++- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/shiny/ui/_include_helpers.py b/shiny/ui/_include_helpers.py index 33840b94b..f5427d5f5 100644 --- a/shiny/ui/_include_helpers.py +++ b/shiny/ui/_include_helpers.py @@ -214,37 +214,61 @@ def create_include_dependency( def maybe_copy_files(path: Path | str, include_files: bool) -> tuple[str, str]: hash = get_hash(path, include_files) + print("path: ", path, "Hash: ", hash) # To avoid unnecessary work when the same file is included multiple times, # use a directory scoped by a hash of the file. tmpdir = os.path.join(tempfile.gettempdir(), "shiny_include_files", hash) path_dest = os.path.join(tmpdir, os.path.basename(path)) + print("path_dest:", path_dest) # Since the hash/tmpdir should represent all the files in the path's directory, - # we can simply return here + # we can check if it exists to determine if we have a cache hit if os.path.exists(path_dest): + print("Already exists", path_dest) return path_dest, hash + permissions = ( + 0o755 # Store permissions var set to -rwxr-xr-x (owner rwx, others rx) + ) + # Otherwise, make sure we have a clean slate if os.path.exists(tmpdir): + print("folder exists, not contents:", os.stat(tmpdir)) shutil.rmtree(tmpdir) + # Looks like this is copying twice? puts stuff in diff folders? if include_files: shutil.copytree(os.path.dirname(path), tmpdir) - else: - os.makedirs(tmpdir, exist_ok=True) - shutil.copy(path, path_dest) + print("new folder, perms: ", oct(os.stat(tmpdir).st_mode)) + for root, dirs, files in os.walk(tmpdir): + # set perms on sub-directories + for dir in dirs: + os.chmod(os.path.join(root, dir), permissions) + print("Dir: ", dir) + print("dir perms: ", oct(os.stat(os.path.join(root, dir)).st_mode)) + + # set perms on files + for file in files: + print("File: ", file) + os.chmod(os.path.join(root, file), permissions) + print("file perms: ", oct(os.stat(os.path.join(root, file)).st_mode)) + shutil.copy(path, path_dest) + print("PATH_DEST: ", path_dest) return path_dest, hash def get_hash(path: Path | str, include_files: bool) -> str: if include_files: + print("get_hash, include_files is true") dir = os.path.dirname(path) files = glob.iglob(os.path.join(dir, "**"), recursive=True) key = "\n".join([get_file_key(x) for x in files]) + print("KEY: ", key) else: key = get_file_key(path) + print("not include key: ", key) return hash_deterministic(key) diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py index 438af56dd..b994a7057 100644 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py @@ -2,7 +2,9 @@ # Define the UI app_ui = ui.page_fluid( - ui.include_css("./www/style.css"), # Reference the external CSS file + ui.include_css( + "./www/style.css", method="link_files" + ), # Reference the external CSS file ui.include_js("./www/customjs.js"), ui.h1("Simple Shiny App with External CSS"), ui.div( From 2764cbe5dc90824fb5cdf23ae123ba9c8bc59d4d Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Tue, 26 Aug 2025 13:31:51 -0400 Subject: [PATCH 05/20] Add multifile example, notes --- shiny/ui/_include_helpers.py | 1 + .../{ => app-lite}/app.py | 0 .../{ => app-lite}/www/customjs.js | 0 .../{ => app-lite}/www/style.css | 0 .../2061-css-js-inclusion/app-multi/app.py | 22 +++++++++++++++++++ .../app-multi/css/more.css | 3 +++ .../app-multi/css/style.css | 19 ++++++++++++++++ .../app-multi/js/customjs.js | 4 ++++ .../app-multi/js/more.js | 3 +++ 9 files changed, 52 insertions(+) rename tests/playwright/shiny/bugs/2061-css-js-inclusion/{ => app-lite}/app.py (100%) rename tests/playwright/shiny/bugs/2061-css-js-inclusion/{ => app-lite}/www/customjs.js (100%) rename tests/playwright/shiny/bugs/2061-css-js-inclusion/{ => app-lite}/www/style.css (100%) create mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/app.py create mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/css/more.css create mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/css/style.css create mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/js/customjs.js create mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/js/more.js diff --git a/shiny/ui/_include_helpers.py b/shiny/ui/_include_helpers.py index f5427d5f5..7b135e887 100644 --- a/shiny/ui/_include_helpers.py +++ b/shiny/ui/_include_helpers.py @@ -19,6 +19,7 @@ # as the app, the app's source will be included. Should we just not copy .py/.r files? +# TODO: This does throw js errors if you're loading in other stuff depending on your config @add_example() def include_js( path: Path | str, diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-lite/app.py similarity index 100% rename from tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py rename to tests/playwright/shiny/bugs/2061-css-js-inclusion/app-lite/app.py diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/www/customjs.js b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-lite/www/customjs.js similarity index 100% rename from tests/playwright/shiny/bugs/2061-css-js-inclusion/www/customjs.js rename to tests/playwright/shiny/bugs/2061-css-js-inclusion/app-lite/www/customjs.js diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/www/style.css b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-lite/www/style.css similarity index 100% rename from tests/playwright/shiny/bugs/2061-css-js-inclusion/www/style.css rename to tests/playwright/shiny/bugs/2061-css-js-inclusion/app-lite/www/style.css diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/app.py b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/app.py new file mode 100644 index 000000000..7a37f8dab --- /dev/null +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/app.py @@ -0,0 +1,22 @@ +from shiny import App, ui + +# Define the UI +app_ui = ui.page_fluid( + ui.include_css("./css/style.css", method="link_files"), + ui.include_js("./js/customjs.js", method="link_files"), + ui.h1("Simple Shiny App with External CSS"), + ui.div( + ui.p("This is a simple Shiny app that demonstrates ui.include_css()"), + ui.p("The styling comes from an external CSS file!"), + class_="content", + ), +) + + +# Define the server +def server(input, output, session): + pass + + +# Create and run the app +app = App(app_ui, server) diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/css/more.css b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/css/more.css new file mode 100644 index 000000000..67039459e --- /dev/null +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/css/more.css @@ -0,0 +1,3 @@ +body { + background-color: #506577; +} \ No newline at end of file diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/css/style.css b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/css/style.css new file mode 100644 index 000000000..60303125d --- /dev/null +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/css/style.css @@ -0,0 +1,19 @@ +@import url("more.css"); + +body { + font-family: Arial, sans-serif; +} + +h1 { + color: #4682b4; + border-bottom: 2px solid #4682b4; + padding-bottom: 10px; +} + +.content { + margin: 20px; + padding: 15px; + background-color: white; + border-radius: 5px; + box-shadow: 0 0 10px rgba(0, 0, 0, 0.1); +} diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/js/customjs.js b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/js/customjs.js new file mode 100644 index 000000000..bdf776034 --- /dev/null +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/js/customjs.js @@ -0,0 +1,4 @@ +const heyo = require('./more.js'); + + +console.log(heyo()) \ No newline at end of file diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/js/more.js b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/js/more.js new file mode 100644 index 000000000..6dade946a --- /dev/null +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/js/more.js @@ -0,0 +1,3 @@ +export function heyo() { + console.log(heyo) +} \ No newline at end of file From 03839f05d8e7cf5d97ed9aff6c5751b423fb33a4 Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Tue, 26 Aug 2025 14:00:40 -0400 Subject: [PATCH 06/20] Fixing types --- tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py index 438af56dd..41696c8ca 100644 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py @@ -1,4 +1,4 @@ -from shiny import App, ui +from shiny import App, Inputs, Outputs, Session, ui # Define the UI app_ui = ui.page_fluid( @@ -14,7 +14,7 @@ # Define the server -def server(input, output, session): +def server(input: Inputs, output: Outputs, session: Session): pass From c1a11a1cd7b0678b6c8ee63781ec3ff54d2694f8 Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Wed, 27 Aug 2025 12:27:52 -0400 Subject: [PATCH 07/20] Updating tests --- tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py | 4 ++-- .../shiny/bugs/2061-css-js-inclusion/css/more.css | 3 +++ .../bugs/2061-css-js-inclusion/{www => css}/style.css | 4 ++-- .../shiny/bugs/2061-css-js-inclusion/js/customjs.js | 4 ++++ .../bugs/2061-css-js-inclusion/test_css-js-inclusion.py | 9 +++++++++ .../shiny/bugs/2061-css-js-inclusion/www/customjs.js | 1 - 6 files changed, 20 insertions(+), 5 deletions(-) create mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/css/more.css rename tests/playwright/shiny/bugs/2061-css-js-inclusion/{www => css}/style.css (84%) create mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/js/customjs.js create mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/test_css-js-inclusion.py delete mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/www/customjs.js diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py index 41696c8ca..847bcc6eb 100644 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py @@ -2,8 +2,8 @@ # Define the UI app_ui = ui.page_fluid( - ui.include_css("./www/style.css"), # Reference the external CSS file - ui.include_js("./www/customjs.js"), + ui.include_css("./css/style.css", method="link_files"), + ui.include_js("./js/customjs.js", method="link_files"), ui.h1("Simple Shiny App with External CSS"), ui.div( ui.p("This is a simple Shiny app that demonstrates ui.include_css()"), diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/css/more.css b/tests/playwright/shiny/bugs/2061-css-js-inclusion/css/more.css new file mode 100644 index 000000000..3eda1891b --- /dev/null +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/css/more.css @@ -0,0 +1,3 @@ +body { + background-color: #c8e1f7; +} \ No newline at end of file diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/www/style.css b/tests/playwright/shiny/bugs/2061-css-js-inclusion/css/style.css similarity index 84% rename from tests/playwright/shiny/bugs/2061-css-js-inclusion/www/style.css rename to tests/playwright/shiny/bugs/2061-css-js-inclusion/css/style.css index e86131d86..810040c7a 100644 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/www/style.css +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/css/style.css @@ -1,11 +1,11 @@ +@import url("more.css"); body { - background-color: #f0f8ff; font-family: Arial, sans-serif; } h1 { - color: #4682b4; + color: black; border-bottom: 2px solid #4682b4; padding-bottom: 10px; } diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/js/customjs.js b/tests/playwright/shiny/bugs/2061-css-js-inclusion/js/customjs.js new file mode 100644 index 000000000..70925a499 --- /dev/null +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/js/customjs.js @@ -0,0 +1,4 @@ +const newParagraph = document.createElement('p'); +newParagraph.textContent = 'Heyo!'; +const bodyElement = document.body; +bodyElement.appendChild(newParagraph); diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/test_css-js-inclusion.py b/tests/playwright/shiny/bugs/2061-css-js-inclusion/test_css-js-inclusion.py new file mode 100644 index 000000000..b144f78f6 --- /dev/null +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/test_css-js-inclusion.py @@ -0,0 +1,9 @@ +from playwright.sync_api import Page, expect + +from shiny.run import ShinyAppProc + + +def test_inclusion(page: Page, local_app: ShinyAppProc) -> None: + page.goto(local_app.url) + + expect(page.locator("body > p")).to_have_text("Heyo!") diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/www/customjs.js b/tests/playwright/shiny/bugs/2061-css-js-inclusion/www/customjs.js deleted file mode 100644 index dbb1b36fa..000000000 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/www/customjs.js +++ /dev/null @@ -1 +0,0 @@ -console.log("heyo") \ No newline at end of file From ff8581eae0efcf4f9ce522187512f2d02f7a51b6 Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Wed, 27 Aug 2025 12:49:29 -0400 Subject: [PATCH 08/20] Debugging --- tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py index 847bcc6eb..aa823bfd0 100644 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py @@ -1,9 +1,13 @@ from shiny import App, Inputs, Outputs, Session, ui +from pathlib import Path + +js_file = Path(__file__).parent / "js" / "customjs.js" +css_file = Path(__file__).parent / "css" / "style.css" # Define the UI app_ui = ui.page_fluid( - ui.include_css("./css/style.css", method="link_files"), - ui.include_js("./js/customjs.js", method="link_files"), + ui.include_css(css_file, method="link_files"), + ui.include_js(js_file, method="link_files"), ui.h1("Simple Shiny App with External CSS"), ui.div( ui.p("This is a simple Shiny app that demonstrates ui.include_css()"), From c888903703f34a9d71e936b363df6a284b4663c2 Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Wed, 27 Aug 2025 12:56:52 -0400 Subject: [PATCH 09/20] fixes --- tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py index aa823bfd0..c8e979769 100644 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py @@ -1,6 +1,7 @@ -from shiny import App, Inputs, Outputs, Session, ui from pathlib import Path +from shiny import App, Inputs, Outputs, Session, ui + js_file = Path(__file__).parent / "js" / "customjs.js" css_file = Path(__file__).parent / "css" / "style.css" From e99f7959230a0e77b5e18423096c1dee6a5e90fb Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Wed, 27 Aug 2025 13:07:08 -0400 Subject: [PATCH 10/20] Deleting old test --- .../2061-css-js-inclusion/app-lite/app.py | 24 ------------------- .../app-lite/www/customjs.js | 1 - .../app-lite/www/style.css | 19 --------------- .../2061-css-js-inclusion/app-multi/app.py | 22 ----------------- .../app-multi/css/more.css | 3 --- .../app-multi/css/style.css | 19 --------------- .../app-multi/js/customjs.js | 4 ---- .../app-multi/js/more.js | 3 --- 8 files changed, 95 deletions(-) delete mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/app-lite/app.py delete mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/app-lite/www/customjs.js delete mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/app-lite/www/style.css delete mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/app.py delete mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/css/more.css delete mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/css/style.css delete mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/js/customjs.js delete mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/js/more.js diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-lite/app.py b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-lite/app.py deleted file mode 100644 index b994a7057..000000000 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-lite/app.py +++ /dev/null @@ -1,24 +0,0 @@ -from shiny import App, ui - -# Define the UI -app_ui = ui.page_fluid( - ui.include_css( - "./www/style.css", method="link_files" - ), # Reference the external CSS file - ui.include_js("./www/customjs.js"), - ui.h1("Simple Shiny App with External CSS"), - ui.div( - ui.p("This is a simple Shiny app that demonstrates ui.include_css()"), - ui.p("The styling comes from an external CSS file!"), - class_="content", - ), -) - - -# Define the server -def server(input, output, session): - pass - - -# Create and run the app -app = App(app_ui, server) diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-lite/www/customjs.js b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-lite/www/customjs.js deleted file mode 100644 index dbb1b36fa..000000000 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-lite/www/customjs.js +++ /dev/null @@ -1 +0,0 @@ -console.log("heyo") \ No newline at end of file diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-lite/www/style.css b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-lite/www/style.css deleted file mode 100644 index e86131d86..000000000 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-lite/www/style.css +++ /dev/null @@ -1,19 +0,0 @@ - -body { - background-color: #f0f8ff; - font-family: Arial, sans-serif; -} - -h1 { - color: #4682b4; - border-bottom: 2px solid #4682b4; - padding-bottom: 10px; -} - -.content { - margin: 20px; - padding: 15px; - background-color: white; - border-radius: 5px; - box-shadow: 0 0 10px rgba(0, 0, 0, 0.1); -} diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/app.py b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/app.py deleted file mode 100644 index 7a37f8dab..000000000 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/app.py +++ /dev/null @@ -1,22 +0,0 @@ -from shiny import App, ui - -# Define the UI -app_ui = ui.page_fluid( - ui.include_css("./css/style.css", method="link_files"), - ui.include_js("./js/customjs.js", method="link_files"), - ui.h1("Simple Shiny App with External CSS"), - ui.div( - ui.p("This is a simple Shiny app that demonstrates ui.include_css()"), - ui.p("The styling comes from an external CSS file!"), - class_="content", - ), -) - - -# Define the server -def server(input, output, session): - pass - - -# Create and run the app -app = App(app_ui, server) diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/css/more.css b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/css/more.css deleted file mode 100644 index 67039459e..000000000 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/css/more.css +++ /dev/null @@ -1,3 +0,0 @@ -body { - background-color: #506577; -} \ No newline at end of file diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/css/style.css b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/css/style.css deleted file mode 100644 index 60303125d..000000000 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/css/style.css +++ /dev/null @@ -1,19 +0,0 @@ -@import url("more.css"); - -body { - font-family: Arial, sans-serif; -} - -h1 { - color: #4682b4; - border-bottom: 2px solid #4682b4; - padding-bottom: 10px; -} - -.content { - margin: 20px; - padding: 15px; - background-color: white; - border-radius: 5px; - box-shadow: 0 0 10px rgba(0, 0, 0, 0.1); -} diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/js/customjs.js b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/js/customjs.js deleted file mode 100644 index bdf776034..000000000 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/js/customjs.js +++ /dev/null @@ -1,4 +0,0 @@ -const heyo = require('./more.js'); - - -console.log(heyo()) \ No newline at end of file diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/js/more.js b/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/js/more.js deleted file mode 100644 index 6dade946a..000000000 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app-multi/js/more.js +++ /dev/null @@ -1,3 +0,0 @@ -export function heyo() { - console.log(heyo) -} \ No newline at end of file From 14012c8815eb81bca28fe39115aaff8565621e3c Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Thu, 28 Aug 2025 09:57:12 -0400 Subject: [PATCH 11/20] Added comments, minor changes --- shiny/ui/_include_helpers.py | 43 ++++++++----------- .../{ => include_files_case}/app.py | 0 .../css/evenmorecss}/more.css | 0 .../include_files_case/css/style.css | 19 ++++++++ .../{ => include_files_case}/js/customjs.js | 0 .../test_css-js-inclusion.py | 0 .../not_include_files_case/app.py | 27 ++++++++++++ .../not_include_files_case/customjs.js | 4 ++ .../{css => not_include_files_case}/style.css | 3 +- .../test_css-js-inclusion.py | 9 ++++ 10 files changed, 79 insertions(+), 26 deletions(-) rename tests/playwright/shiny/bugs/2061-css-js-inclusion/{ => include_files_case}/app.py (100%) rename tests/playwright/shiny/bugs/2061-css-js-inclusion/{css => include_files_case/css/evenmorecss}/more.css (100%) create mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/css/style.css rename tests/playwright/shiny/bugs/2061-css-js-inclusion/{ => include_files_case}/js/customjs.js (100%) rename tests/playwright/shiny/bugs/2061-css-js-inclusion/{ => include_files_case}/test_css-js-inclusion.py (100%) create mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/app.py create mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/customjs.js rename tests/playwright/shiny/bugs/2061-css-js-inclusion/{css => not_include_files_case}/style.css (90%) create mode 100644 tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/test_css-js-inclusion.py diff --git a/shiny/ui/_include_helpers.py b/shiny/ui/_include_helpers.py index 7b135e887..28a3d4c4e 100644 --- a/shiny/ui/_include_helpers.py +++ b/shiny/ui/_include_helpers.py @@ -226,50 +226,45 @@ def maybe_copy_files(path: Path | str, include_files: bool) -> tuple[str, str]: # Since the hash/tmpdir should represent all the files in the path's directory, # we can check if it exists to determine if we have a cache hit if os.path.exists(path_dest): - print("Already exists", path_dest) + print("Path already exists:", path_dest) return path_dest, hash - permissions = ( - 0o755 # Store permissions var set to -rwxr-xr-x (owner rwx, others rx) - ) - # Otherwise, make sure we have a clean slate if os.path.exists(tmpdir): - print("folder exists, not contents:", os.stat(tmpdir)) + print("Folder already exists, but not files, removing.") shutil.rmtree(tmpdir) - # Looks like this is copying twice? puts stuff in diff folders? + # This recursively changes permissions to 755 (owner rwx, other rx) for all files + # under `tmp/unique_hash` so that the app can be run by other collaborators + # on a multi-tenant system. See #2061 if include_files: shutil.copytree(os.path.dirname(path), tmpdir) - print("new folder, perms: ", oct(os.stat(tmpdir).st_mode)) - for root, dirs, files in os.walk(tmpdir): - # set perms on sub-directories - for dir in dirs: - os.chmod(os.path.join(root, dir), permissions) - print("Dir: ", dir) - print("dir perms: ", oct(os.stat(os.path.join(root, dir)).st_mode)) - + for dirpath, dirs, filenames in os.walk(tmpdir): # set perms on files - for file in files: - print("File: ", file) - os.chmod(os.path.join(root, file), permissions) - print("file perms: ", oct(os.stat(os.path.join(root, file)).st_mode)) - + for file in filenames: + os.chmod(os.path.join(dirpath, file), 0o755) + print( + "File: ", + file, + "File perms: ", + oct(os.stat(os.path.join(dirpath, file)).st_mode), + ) + else: + os.mkdir(tmpdir, mode=0o755) shutil.copy(path, path_dest) - print("PATH_DEST: ", path_dest) + os.chmod(path_dest, 0o755) + print("File: ", path_dest, "perms", oct(os.stat(path_dest).st_mode)) + return path_dest, hash def get_hash(path: Path | str, include_files: bool) -> str: if include_files: - print("get_hash, include_files is true") dir = os.path.dirname(path) files = glob.iglob(os.path.join(dir, "**"), recursive=True) key = "\n".join([get_file_key(x) for x in files]) - print("KEY: ", key) else: key = get_file_key(path) - print("not include key: ", key) return hash_deterministic(key) diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py b/tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/app.py similarity index 100% rename from tests/playwright/shiny/bugs/2061-css-js-inclusion/app.py rename to tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/app.py diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/css/more.css b/tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/css/evenmorecss/more.css similarity index 100% rename from tests/playwright/shiny/bugs/2061-css-js-inclusion/css/more.css rename to tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/css/evenmorecss/more.css diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/css/style.css b/tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/css/style.css new file mode 100644 index 000000000..5d9a8e294 --- /dev/null +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/css/style.css @@ -0,0 +1,19 @@ +@import url("./evenmorecss/more.css"); + +body { + font-family: Arial, sans-serif; +} + +h1 { + color: black; + border-bottom: 2px solid #4682b4; + padding-bottom: 10px; +} + +.content { + margin: 20px; + padding: 15px; + background-color: white; + border-radius: 5px; + box-shadow: 0 0 10px rgba(0, 0, 0, 0.1); +} diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/js/customjs.js b/tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/js/customjs.js similarity index 100% rename from tests/playwright/shiny/bugs/2061-css-js-inclusion/js/customjs.js rename to tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/js/customjs.js diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/test_css-js-inclusion.py b/tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/test_css-js-inclusion.py similarity index 100% rename from tests/playwright/shiny/bugs/2061-css-js-inclusion/test_css-js-inclusion.py rename to tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/test_css-js-inclusion.py diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/app.py b/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/app.py new file mode 100644 index 000000000..bfccf95fb --- /dev/null +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/app.py @@ -0,0 +1,27 @@ +from pathlib import Path + +from shiny import App, Inputs, Outputs, Session, ui + +js_file = Path(__file__).parent / "customjs.js" +css_file = Path(__file__).parent / "style.css" + +# Define the UI +app_ui = ui.page_fluid( + ui.include_css(css_file, method="link"), + ui.include_js(js_file, method="inline"), + ui.h1("Simple Shiny App with External CSS"), + ui.div( + ui.p("This is a simple Shiny app that demonstrates ui.include_css()"), + ui.p("The styling comes from an external CSS file!"), + class_="content", + ), +) + + +# Define the server +def server(input: Inputs, output: Outputs, session: Session): + pass + + +# Create and run the app +app = App(app_ui, server) diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/customjs.js b/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/customjs.js new file mode 100644 index 000000000..70925a499 --- /dev/null +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/customjs.js @@ -0,0 +1,4 @@ +const newParagraph = document.createElement('p'); +newParagraph.textContent = 'Heyo!'; +const bodyElement = document.body; +bodyElement.appendChild(newParagraph); diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/css/style.css b/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/style.css similarity index 90% rename from tests/playwright/shiny/bugs/2061-css-js-inclusion/css/style.css rename to tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/style.css index 810040c7a..078238b67 100644 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/css/style.css +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/style.css @@ -1,6 +1,5 @@ -@import url("more.css"); - body { + background-color: #70bfef; font-family: Arial, sans-serif; } diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/test_css-js-inclusion.py b/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/test_css-js-inclusion.py new file mode 100644 index 000000000..b144f78f6 --- /dev/null +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/test_css-js-inclusion.py @@ -0,0 +1,9 @@ +from playwright.sync_api import Page, expect + +from shiny.run import ShinyAppProc + + +def test_inclusion(page: Page, local_app: ShinyAppProc) -> None: + page.goto(local_app.url) + + expect(page.locator("body > p")).to_have_text("Heyo!") From 7ffeab5cce0c8c9b7d30023eff3c1a347cfcc934 Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Thu, 28 Aug 2025 11:45:18 -0400 Subject: [PATCH 12/20] Relinking vs inline --- .../bugs/2061-css-js-inclusion/not_include_files_case/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/app.py b/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/app.py index bfccf95fb..4072ca609 100644 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/app.py +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/app.py @@ -8,7 +8,7 @@ # Define the UI app_ui = ui.page_fluid( ui.include_css(css_file, method="link"), - ui.include_js(js_file, method="inline"), + ui.include_js(js_file, method="link"), ui.h1("Simple Shiny App with External CSS"), ui.div( ui.p("This is a simple Shiny app that demonstrates ui.include_css()"), From 63a3c40a09e49ff29da831b8f0e8b8285763bc50 Mon Sep 17 00:00:00 2001 From: E Nelson Date: Thu, 28 Aug 2025 16:43:25 -0400 Subject: [PATCH 13/20] Update shiny/ui/_include_helpers.py Co-authored-by: Carson Sievert --- shiny/ui/_include_helpers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/shiny/ui/_include_helpers.py b/shiny/ui/_include_helpers.py index 28a3d4c4e..49b2c8a14 100644 --- a/shiny/ui/_include_helpers.py +++ b/shiny/ui/_include_helpers.py @@ -19,7 +19,6 @@ # as the app, the app's source will be included. Should we just not copy .py/.r files? -# TODO: This does throw js errors if you're loading in other stuff depending on your config @add_example() def include_js( path: Path | str, From 6609d20e82f5a88d2dd801974f01af63b4fc4ba9 Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Thu, 28 Aug 2025 16:48:00 -0400 Subject: [PATCH 14/20] Updating prints --- shiny/ui/_include_helpers.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/shiny/ui/_include_helpers.py b/shiny/ui/_include_helpers.py index 49b2c8a14..5ac87189d 100644 --- a/shiny/ui/_include_helpers.py +++ b/shiny/ui/_include_helpers.py @@ -249,7 +249,13 @@ def maybe_copy_files(path: Path | str, include_files: bool) -> tuple[str, str]: oct(os.stat(os.path.join(dirpath, file)).st_mode), ) else: - os.mkdir(tmpdir, mode=0o755) + os.makedirs(tmpdir, mode=0o755, exist_ok=True) + print( + "Copying files from: ", + path, + " with perms: ", + oct(os.stat(path).st_mode), + ) shutil.copy(path, path_dest) os.chmod(path_dest, 0o755) print("File: ", path_dest, "perms", oct(os.stat(path_dest).st_mode)) From b477812db581590468ff079bc49780fb0e899452 Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Fri, 29 Aug 2025 16:57:28 -0400 Subject: [PATCH 15/20] Updating 'shiny_include_files to have 777 perms --- shiny/ui/_include_helpers.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/shiny/ui/_include_helpers.py b/shiny/ui/_include_helpers.py index 5ac87189d..0da403f68 100644 --- a/shiny/ui/_include_helpers.py +++ b/shiny/ui/_include_helpers.py @@ -218,9 +218,14 @@ def maybe_copy_files(path: Path | str, include_files: bool) -> tuple[str, str]: # To avoid unnecessary work when the same file is included multiple times, # use a directory scoped by a hash of the file. - tmpdir = os.path.join(tempfile.gettempdir(), "shiny_include_files", hash) + tmp_folder = os.path.join( + os.path.basename(path), tempfile.gettempdir(), "shiny_include_files" + ) + tmpdir = os.path.join(tmp_folder, hash) path_dest = os.path.join(tmpdir, os.path.basename(path)) print("path_dest:", path_dest) + print(tmp_folder) + os.chmod(tmp_folder, 0o755) # Since the hash/tmpdir should represent all the files in the path's directory, # we can check if it exists to determine if we have a cache hit From 30fc4f78f72b4e5772a5231814cdb586c5aa2b57 Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Tue, 2 Sep 2025 16:44:59 -0400 Subject: [PATCH 16/20] Testing only new dir change --- shiny/ui/_include_helpers.py | 42 ++++++++++++------------------------ 1 file changed, 14 insertions(+), 28 deletions(-) diff --git a/shiny/ui/_include_helpers.py b/shiny/ui/_include_helpers.py index 0da403f68..f63b2b122 100644 --- a/shiny/ui/_include_helpers.py +++ b/shiny/ui/_include_helpers.py @@ -216,17 +216,12 @@ def maybe_copy_files(path: Path | str, include_files: bool) -> tuple[str, str]: hash = get_hash(path, include_files) print("path: ", path, "Hash: ", hash) - # To avoid unnecessary work when the same file is included multiple times, - # use a directory scoped by a hash of the file. - tmp_folder = os.path.join( - os.path.basename(path), tempfile.gettempdir(), "shiny_include_files" - ) - tmpdir = os.path.join(tmp_folder, hash) + tmpdir = os.path.join(tempfile.gettempdir(), f"shiny_include_files_{hash}") path_dest = os.path.join(tmpdir, os.path.basename(path)) - print("path_dest:", path_dest) - print(tmp_folder) - os.chmod(tmp_folder, 0o755) + print("tmpdir: ", tmpdir, "path_dest: ", path_dest) + # To avoid unnecessary work when the same file is included multiple times, + # use a directory scoped by a hash of the file. # Since the hash/tmpdir should represent all the files in the path's directory, # we can check if it exists to determine if we have a cache hit if os.path.exists(path_dest): @@ -234,27 +229,21 @@ def maybe_copy_files(path: Path | str, include_files: bool) -> tuple[str, str]: return path_dest, hash # Otherwise, make sure we have a clean slate - if os.path.exists(tmpdir): + if os.path.exists(path_dest): print("Folder already exists, but not files, removing.") - shutil.rmtree(tmpdir) + shutil.rmtree(path_dest) - # This recursively changes permissions to 755 (owner rwx, other rx) for all files - # under `tmp/unique_hash` so that the app can be run by other collaborators - # on a multi-tenant system. See #2061 if include_files: + print( + "Copying all included files from: ", + path, + " with perms: ", + oct(os.stat(path).st_mode), + ) shutil.copytree(os.path.dirname(path), tmpdir) - for dirpath, dirs, filenames in os.walk(tmpdir): - # set perms on files - for file in filenames: - os.chmod(os.path.join(dirpath, file), 0o755) - print( - "File: ", - file, - "File perms: ", - oct(os.stat(os.path.join(dirpath, file)).st_mode), - ) + else: - os.makedirs(tmpdir, mode=0o755, exist_ok=True) + os.makedirs(path_dest, mode=0o755, exist_ok=True) print( "Copying files from: ", path, @@ -262,9 +251,6 @@ def maybe_copy_files(path: Path | str, include_files: bool) -> tuple[str, str]: oct(os.stat(path).st_mode), ) shutil.copy(path, path_dest) - os.chmod(path_dest, 0o755) - print("File: ", path_dest, "perms", oct(os.stat(path_dest).st_mode)) - return path_dest, hash From 87afb30d110fb836490d7242c33e6f918f92e49c Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Wed, 3 Sep 2025 15:23:55 -0400 Subject: [PATCH 17/20] Updating to deal with 404 --- shiny/ui/_include_helpers.py | 8 +++++--- .../include_files_case/css/style.css | 2 ++ .../include_files_case/js/customjs.js | 4 ++-- .../not_include_files_case/customjs.js | 3 +-- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/shiny/ui/_include_helpers.py b/shiny/ui/_include_helpers.py index f63b2b122..57d9ca180 100644 --- a/shiny/ui/_include_helpers.py +++ b/shiny/ui/_include_helpers.py @@ -197,6 +197,7 @@ def check_path(path: Path | str) -> Path: def create_include_dependency( name: str, path: str, include_files: bool ) -> tuple[HTMLDependency, str]: + print("create include: ", name, os.path.dirname(path)) dep = HTMLDependency( name, DEFAULT_VERSION, @@ -216,7 +217,7 @@ def maybe_copy_files(path: Path | str, include_files: bool) -> tuple[str, str]: hash = get_hash(path, include_files) print("path: ", path, "Hash: ", hash) - tmpdir = os.path.join(tempfile.gettempdir(), f"shiny_include_files_{hash}") + tmpdir = os.path.join(tempfile.gettempdir(), f"shiny_include_{hash}") path_dest = os.path.join(tmpdir, os.path.basename(path)) print("tmpdir: ", tmpdir, "path_dest: ", path_dest) @@ -229,7 +230,7 @@ def maybe_copy_files(path: Path | str, include_files: bool) -> tuple[str, str]: return path_dest, hash # Otherwise, make sure we have a clean slate - if os.path.exists(path_dest): + if os.path.exists(tmpdir): print("Folder already exists, but not files, removing.") shutil.rmtree(path_dest) @@ -243,7 +244,7 @@ def maybe_copy_files(path: Path | str, include_files: bool) -> tuple[str, str]: shutil.copytree(os.path.dirname(path), tmpdir) else: - os.makedirs(path_dest, mode=0o755, exist_ok=True) + os.makedirs(tmpdir, exist_ok=True) print( "Copying files from: ", path, @@ -251,6 +252,7 @@ def maybe_copy_files(path: Path | str, include_files: bool) -> tuple[str, str]: oct(os.stat(path).st_mode), ) shutil.copy(path, path_dest) + return path_dest, hash diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/css/style.css b/tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/css/style.css index 5d9a8e294..99c3ee888 100644 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/css/style.css +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/css/style.css @@ -17,3 +17,5 @@ h1 { border-radius: 5px; box-shadow: 0 0 10px rgba(0, 0, 0, 0.1); } + + diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/js/customjs.js b/tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/js/customjs.js index 70925a499..4dc8fcc69 100644 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/js/customjs.js +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/include_files_case/js/customjs.js @@ -1,4 +1,4 @@ const newParagraph = document.createElement('p'); newParagraph.textContent = 'Heyo!'; -const bodyElement = document.body; -bodyElement.appendChild(newParagraph); +document.body.appendChild(newParagraph); + diff --git a/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/customjs.js b/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/customjs.js index 70925a499..c7e8ae251 100644 --- a/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/customjs.js +++ b/tests/playwright/shiny/bugs/2061-css-js-inclusion/not_include_files_case/customjs.js @@ -1,4 +1,3 @@ const newParagraph = document.createElement('p'); newParagraph.textContent = 'Heyo!'; -const bodyElement = document.body; -bodyElement.appendChild(newParagraph); +document.body.appendChild(newParagraph); From 8ae64602b56d3aeabcfc413f6542ef39d75ad425 Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Wed, 3 Sep 2025 15:25:13 -0400 Subject: [PATCH 18/20] Updating one missing piece --- shiny/ui/_include_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shiny/ui/_include_helpers.py b/shiny/ui/_include_helpers.py index 57d9ca180..ddaf1ac2c 100644 --- a/shiny/ui/_include_helpers.py +++ b/shiny/ui/_include_helpers.py @@ -232,7 +232,7 @@ def maybe_copy_files(path: Path | str, include_files: bool) -> tuple[str, str]: # Otherwise, make sure we have a clean slate if os.path.exists(tmpdir): print("Folder already exists, but not files, removing.") - shutil.rmtree(path_dest) + shutil.rmtree(tmpdir) if include_files: print( From 64f978e5861f1fb41ba102551aed09af449b641c Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Wed, 3 Sep 2025 16:22:32 -0400 Subject: [PATCH 19/20] Adding del --- shiny/_app.py | 13 +++++++++++++ shiny/ui/_include_helpers.py | 1 - 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/shiny/_app.py b/shiny/_app.py index 0de886a69..a569883f0 100644 --- a/shiny/_app.py +++ b/shiny/_app.py @@ -2,6 +2,7 @@ import copy import os +import shutil import secrets from contextlib import AsyncExitStack, asynccontextmanager from inspect import signature @@ -214,6 +215,18 @@ def __init__( cast("Tag | TagList", ui), lib_prefix=self.lib_prefix ) + def __del__(self): + user_dependencies = [ + v.source["subdir"] + for k, v in self._registered_dependencies.items() + if k.startswith("include-") + ] + + for item in user_dependencies: + if os.path.exists(item): + print("Removing dependency: ", item) + shutil.rmtree(item) + def init_starlette_app(self) -> starlette.applications.Starlette: routes: list[starlette.routing.BaseRoute] = [ starlette.routing.WebSocketRoute("/websocket/", self._on_connect_cb), diff --git a/shiny/ui/_include_helpers.py b/shiny/ui/_include_helpers.py index ddaf1ac2c..3c7fa923e 100644 --- a/shiny/ui/_include_helpers.py +++ b/shiny/ui/_include_helpers.py @@ -197,7 +197,6 @@ def check_path(path: Path | str) -> Path: def create_include_dependency( name: str, path: str, include_files: bool ) -> tuple[HTMLDependency, str]: - print("create include: ", name, os.path.dirname(path)) dep = HTMLDependency( name, DEFAULT_VERSION, From c3e3e22aadb8d07e3018451e3c0c5cca642665f4 Mon Sep 17 00:00:00 2001 From: Liz Nelson Date: Wed, 3 Sep 2025 17:03:00 -0400 Subject: [PATCH 20/20] adding check to make sure it's in the temp dir --- shiny/_app.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/shiny/_app.py b/shiny/_app.py index a569883f0..835e93a10 100644 --- a/shiny/_app.py +++ b/shiny/_app.py @@ -3,6 +3,7 @@ import copy import os import shutil +import tempfile import secrets from contextlib import AsyncExitStack, asynccontextmanager from inspect import signature @@ -216,6 +217,8 @@ def __init__( ) def __del__(self): + current_temp_dir = os.path.realpath(tempfile.gettempdir()) + user_dependencies = [ v.source["subdir"] for k, v in self._registered_dependencies.items() @@ -223,7 +226,12 @@ def __del__(self): ] for item in user_dependencies: - if os.path.exists(item): + # Only remove the item if it exists and it is in the current temp directory + if ( + os.path.exists(item) + and os.path.commonprefix([os.path.realpath(item), current_temp_dir]) + == current_temp_dir + ): print("Removing dependency: ", item) shutil.rmtree(item)