Skip to content

Conversation

@cscheid
Copy link
Collaborator

@cscheid cscheid commented Dec 16, 2025

This should close a few of our quarto preview intermittent issues whose symptom is a missing quarto_ipynb file.

See, eg, #12992.

@posit-snyk-bot
Copy link
Collaborator

posit-snyk-bot commented Dec 16, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@cscheid
Copy link
Collaborator Author

cscheid commented Dec 16, 2025

This last failing test is interesting. If I'm reading the messages correctly, we previously had:

  • a test that used to pass but which requested invalid options
  • a code path that verified the validity of options that wasn't getting called because of incorrect plumbing of the ProjectContext object

Now that things are properly connected, the test fails. But it seems that this test shouldn't really even be there?

@cderv cderv self-assigned this Jan 8, 2026
@cderv cderv force-pushed the bugfix/quarto-preview-ipynb branch from f89c530 to 6db3515 Compare January 14, 2026 15:53
Copy link
Collaborator

@cderv cderv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test failure in render-output-dir.test.ts (and the other playwright docs/playwright/embed-resources/issue-11860/main.qmd) is caused by a logic ordering issue at:

let context = pContext || (await projectContext(path, nbContext, options)) ||
(await singleFileProjectContext(path, nbContext, options));
// Create a synthetic project when --output-dir is used without a project file
// This creates a temporary .quarto directory to manage the render, which must
// be fully cleaned up afterward to avoid leaving debris (see #9745)
if (!context && options.flags?.outputDir) {
context = await projectContextForDirectory(path, nbContext, options);
// forceClean signals this is a synthetic project that needs full cleanup
// including removing the .quarto scratch directory after rendering (#13625)
options.forceClean = options.flags.clean !== false;
}

The change on lines 51-52 now always creates a singleFileProjectContext as fallback:

let context = pContext || (await projectContext(path, nbContext, options)) ||
(await singleFileProjectContext(path, nbContext, options));

This means context is never null when the --output-dir check runs on line 57:

if (!context && options.flags?.outputDir) {

The synthetic project creation for --output-dir is now unreachable. The fix could be to check for --output-dir before falling back to singleFileProjectContext, if we really need this fallback. I am unsure about that and its relation to preview fix which is the topic of this PR.

For example

diff --git a/src/command/render/render-shared.ts b/src/command/render/render-shared.ts
--- a/src/command/render/render-shared.ts
+++ b/src/command/render/render-shared.ts
@@ -48,9 +48,7 @@ export async function render(
   const nbContext = pContext?.notebookContext || notebookContext();

   // determine target context/files
-  // let context = await projectContext(path, nbContext, options);
-  let context = pContext || (await projectContext(path, nbContext, options)) ||
-    (await singleFileProjectContext(path, nbContext, options));
+  let context = pContext || (await projectContext(path, nbContext, options));

   // Create a synthetic project when --output-dir is used without a project file
   // This creates a temporary .quarto directory to manage the render, which must
@@ -62,6 +60,11 @@ export async function render(
     options.forceClean = options.flags.clean !== false;
   }

+  // Only fall back to singleFileProjectContext if we don't have a context yet
+  if (!context) {
+    context = await singleFileProjectContext(path, nbContext, options);
+  }
+
   // set env var if requested
   if (context && options.setProjectDir) {
     // FIXME we can't set environment variables like this with asyncs flying around

# --enable-experimental-regexp-engine is required for /regex/l, https://github.com/quarto-dev/quarto-cli/issues/9737
if [ "$QUARTO_DENO_V8_OPTIONS" != "" ]; then
QUARTO_DENO_V8_OPTIONS="--enable-experimental-regexp-engine,--max-old-space-size=8192,--max-heap-size=8192,${QUARTO_DENO_V8_OPTIONS}"
QUARTO_DENO_V8_OPTIONS="--enable-experimental-regexp-engine,--max-old-space-size=8192,--max-heap-size=8192,--stack-trace-limit=100,${QUARTO_DENO_V8_OPTIONS}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this flag ?

Just to understand. Because we probably need to do the same in quarto.cmd for the windows version to keep both scripts in sync.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It helps with debugging and shouldn't otherwise hurt. It's not important.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I'll add it. Otherwise we don't know why both script are differents.

Comment on lines +50 to +52
// let context = await projectContext(path, nbContext, options);
let context = pContext || (await projectContext(path, nbContext, options)) ||
(await singleFileProjectContext(path, nbContext, options));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the change that makes a test fail. Before we add context = null possible, which lead to the if (!context && options.flags?.outputDir) for synthetic project creation.

Now this does not go there, and --output-dir can't work with singleFileProjectContext()

Is this expected to fix the preview problem that we fallback to singleFileProject here ?
Asking to know what is the fix we should do here: just reorder the logic and fallback, or still allow context = null ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants