From 3cb8c37553c2e1f98ba26cb8a52661c33e215f72 Mon Sep 17 00:00:00 2001
From: MichaelHatherly <michaelhatherly@gmail.com>
Date: Thu, 11 Jan 2024 17:44:57 +0000
Subject: [PATCH] Fix package integration hooks logic

Only attempt to rerun hooks for packages that have actually already been loaded, otherwise version checks will fail since the package isn't actually loaded. Cache package version as an optimisation.
---
 src/server.jl                             |  2 +-
 src/worker.jl                             | 26 +++++++++++++------
 test/examples/integrations/CairoMakie.qmd |  2 ++
 test/runtests.jl                          | 31 +++++++++++++++++++++++
 4 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/src/server.jl b/src/server.jl
index 0c20f50..a84e687 100644
--- a/src/server.jl
+++ b/src/server.jl
@@ -184,7 +184,7 @@ function raw_markdown_chunks(path::String)
             )
         end
 
-        frontmatter = _recursive_merge(CommonMark.frontmatter(ast), default_frontmatter())
+        frontmatter = _recursive_merge(default_frontmatter(), CommonMark.frontmatter(ast))
 
         return raw_chunks, frontmatter
     else
diff --git a/src/worker.jl b/src/worker.jl
index 3ef71d3..ff09032 100644
--- a/src/worker.jl
+++ b/src/worker.jl
@@ -69,8 +69,10 @@ function worker_init(f::File)
             # Rerun the package loading hooks if the frontmatter has changed.
             if FRONTMATTER[] != frontmatter
                 FRONTMATTER[] = frontmatter
-                for hook in values(PACKAGE_LOADING_HOOKS)
-                    hook()
+                for (pkgid, hook) in PACKAGE_LOADING_HOOKS
+                    if haskey(Base.loaded_modules, pkgid)
+                        hook()
+                    end
                 end
             else
                 FRONTMATTER[] = frontmatter
@@ -291,12 +293,20 @@ function worker_init(f::File)
             return (; fig_width, fig_height, fig_format, fig_dpi)
         end
 
+        const PKG_VERSIONS = Dict{Base.PkgId,VersionNumber}()
         function _pkg_version(pkgid::Base.PkgId)
-            deps = Pkg.dependencies()
-            if haskey(deps, pkgid.uuid)
-                return deps[pkgid.uuid].version
+            # Cache the package versions since once a version of a package is
+            # loaded we don't really support loading a different version of it,
+            # so we can just cache the version number.
+            if haskey(PKG_VERSIONS, pkgid)
+                return PKG_VERSIONS[pkgid]
             else
-                return nothing
+                deps = Pkg.dependencies()
+                if haskey(deps, pkgid.uuid)
+                    return PKG_VERSIONS[pkgid] = deps[pkgid.uuid].version
+                else
+                    return nothing
+                end
             end
         end
 
@@ -344,8 +354,8 @@ function worker_init(f::File)
                         mod = get(Base.loaded_modules, pkgid, nothing)
                         try
                             Base.@invokelatest f(pkgid, mod)
-                        catch
-                            @error "hook failed" pkgid mod
+                        catch error
+                            @error "hook failed" pkgid mod error
                         end
                         return nothing
                     end
diff --git a/test/examples/integrations/CairoMakie.qmd b/test/examples/integrations/CairoMakie.qmd
index 4b12571..07f3955 100644
--- a/test/examples/integrations/CairoMakie.qmd
+++ b/test/examples/integrations/CairoMakie.qmd
@@ -1,5 +1,7 @@
 ---
 title: CairoMakie integration
+fig-width: 4
+fig-height: 3
 ---
 
 ```{julia}
diff --git a/test/runtests.jl b/test/runtests.jl
index cb709ef..d39b3b3 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -831,4 +831,35 @@ end
             end
         end
     end
+
+    @testset "package integration hooks" begin
+        env_dir = joinpath(@__DIR__, "examples/integrations/CairoMakie")
+        content = read(joinpath(@__DIR__, "examples/integrations/CairoMakie.qmd"), String)
+        mktempdir() do dir
+            cd(dir) do
+                server = QuartoNotebookRunner.Server()
+
+                cp(env_dir, joinpath(dir, "CairoMakie"))
+                write("CairoMakie.qmd", content)
+
+                json = QuartoNotebookRunner.run!(server, "CairoMakie.qmd")
+
+                image_png = json.cells[end].outputs[1].metadata["image/png"]
+                @test image_png.width == 768
+                @test image_png.height == 576
+
+                content = replace(content, "fig-width: 4" => "fig-width: 8")
+                content = replace(content, "fig-height: 3" => "fig-height: 6")
+                write("CairoMakie.qmd", content)
+
+                json = QuartoNotebookRunner.run!(server, "CairoMakie.qmd")
+
+                image_png = json.cells[end].outputs[1].metadata["image/png"]
+                @test image_png.width == 1536
+                @test image_png.height == 1152
+
+                close!(server)
+            end
+        end
+    end
 end