Repository navigation
Document the OJS architecture and how to update the runtime - #15074
Merged
Merged
Conversation
OJS had no architecture doc, so agents had to rediscover the compile path, the inline-vs-linked runtime injection, and which files under src/resources/formats/html/ojs are generated. Now that the runtime sources live in packages/ojs, the rebuild rule (change the source, rebuild both bundles, commit them together) is also something an agent needs up front. The doc records the invariant that the bundle must not contain a closing script tag, since self-contained renders inline it unescaped, and notes that only the unminified bundle is referenced by quarto-cli.
packages/ojs/README.md explains the layout, build and forks, but not the whole change procedure: bumping the runtime version so the bundle banner identifies the build, committing source and both bundles together, and what to check before merging. The checks cover the paths CI's rebuild comparison doesn't: the inlined self-contained bundle, the Shiny button styling that depends on the Inputs class name, and the MIME types users see through FileAttachment after a mime upgrade. The guide links the README instead of repeating it, and upgrade-dependencies.md points to it because the OJS dependencies are built rather than downloaded like the other HTML dependencies.
vendored-html-dependencies.md applies to src/resources/formats/html/** and says every directory there is managed by update-html-dependencies.ts and never hand-edited. That is wrong for ojs/: the runtime bundles are built from packages/ojs and quarto-ojs.css is maintained in place, so an agent following the rule would look for the bundles in the updater. The new ojs.md rule is scoped to the OJS sources and bundles. It states the rebuild rule and points to the llm-doc and the maintainer guide, which is the discovery pointer llm-docs-maintenance.md asks for.
Checked the OJS architecture doc and update guide against main after the runtime move landed: - prepare-dist copies all of src/resources/, so the minified bundle ships in the distribution even though nothing references it. - Plural server.ojs-exports/ojs-imports keys throw, they do not warn. - Local .js imports are read unchanged; esbuild only transpiles .ts/.tsx imports and bundles .js for self-contained output. - Only the static knitr ojs_define() has the cached-chunk check. - Terser keeps the escape-html /*! notice in the minified bundle. - main_commit now points at the merge that added packages/ojs.
In self-contained renders, compile.ts passes the attachment data URLs to setLocalResolver(), which also fills the resolver that fileAttachmentPathResolver() checks first. A hit returns a plain data URL string, so FileAttachment(...).mimeType is undefined and the bundled mime package is not consulted. The mime-upgrade check therefore has to use a non-self-contained render.
Collaborator
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#15046 moved the OJS runtime and its Observable forks into
packages/ojs/. The README there covers the layout and the build, but nothing in the repo explained how{ojs}cells reach that runtime, or what a runtime change has to go through before it is complete.llm-docs/ojs-architecture.mddescribes the render path:ojsCompile(), how the runtime is linked or inlined for self-contained output, the Lua filter for inline${...},ojs_defineon each engine, resource extraction, and which files are generated vs hand-edited.dev-docs/update-ojs-runtime.mdis the maintainer procedure for a runtime change, an npm dependency bump, or a fork change: version bump, rebuild, what to commit together, and what to verify that the bundle CI check does not cover (self-contained render, Shiny inputs,FileAttachmentMIME types after amimeupgrade).dev-docs/upgrade-dependencies.mdlinks to it..claude/rules/formats/ojs.mdis a path-scoped rule pointing to both docs when editing OJS sources.vendored-html-dependencies.mdnow excludesojs/, sinceupdate-html-dependencies.tsdoes not manage it and the bundles are built frompackages/ojs.The docs were checked against the code on
mainafter #15046 merged.Checklist
I have (if applicable):
AI-assisted PR
Note: autonomous AI agents submitting PRs without human oversight are not permitted — see the Code of Conduct.