Repository navigation
feat(engine): stop preloading docs-nav on every page; one documented registration - #108
Merged
Merged
Conversation
The auto-pin took importmap-rails' default `preload: true`, so in a host whose docs share the application's import map every page carried a modulepreload for docs_nav_controller.js, and a plain host pin could not override it (pin_all_from directories expand over plain pins). Two engine options, read by config/importmap.rb at draw time: - config.docs_kit.preload (default false): the pin's preload:. true restores 1.4's behaviour; an entry-point name preloads it only where that entry point is rendered (importmap-rails 2.x). - config.docs_kit.controllers_under (default "docs_kit/controllers"): the module namespace. "controllers" lets the host's own loader find docs-nav by convention; the asset path never changes. The pin file reads DocsKit::Engine.config.docs_kit, the same shared OrderedOptions as Rails.application.config.docs_kit, so it draws in a spec with no booted app. Refs #107
README gains a "Registering the controller" section (eager, or through
the host's own loader with controllers_under, plus a warning against
lazyLoadControllersFrom("docs_kit/controllers", ...) in a host with its
own controllers). The auto-TOC and dedicated-import-map sections and the
upgrade table point at it. CHANGELOG [Unreleased] names the lost
preload hint and the one-line opt-back.
Refs #107
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
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.
Closes #107
Summary
The engine's auto-pin took importmap-rails' default
preload: true, so every page of a host whose docs share the application's import map carried a<link rel="modulepreload">fordocs_nav_controller.js(24 KB). A plain hostpin … preload: falsecouldn't override it, becausepin_all_fromdirectories expand over plain pins. The README and the pin file's header also gave three different registration recipes, and one of them (lazyLoadControllersFrom("docs_kit/controllers", …)) logs an autoload error for every host controller identifier.config.docs_kit.preload(defaultfalse) sets the pin'spreload:.truerestores 1.4's behaviour. An entry-point name such as"docs"preloads it only where that entry point is rendered (importmap-rails 2.x).config.docs_kit.controllers_under(default"docs_kit/controllers") sets the module namespace. With"controllers"the module becomescontrollers/docs_nav_controllerand the host's own loader finds it by convention: withlazyLoadControllersFrom("controllers", …), only on pages that renderdocs-nav. The asset path doesn't change.config/importmap.rbreads both fromDocsKit::Engine.config.docs_kit. Its header now points at a new README section, Registering the controller, which covers eager registration, registration through the host's loader, preloading, and a warning against a second lazy loader ondocs_kit/controllers. The auto-TOC section, the dedicated-import-map section and the upgrade table point at it.[Unreleased]has an "Upgrading from 1.4" note. The one visible change for an eager-registering site: docs-nav comes from the eager loader'simport()instead of the preload cache, one request later. To get the preload back, setconfig.docs_kit.preload = true.Before / after:
<head>of/docs/overviewin the dogfood app (defaults)<link rel="modulepreload" href="/assets/stimulus-loading-1fc53fe7.js"> -<link rel="modulepreload" href="/assets/docs_kit/controllers/docs_nav_controller-2c0d75d3.js"> <link rel="modulepreload" href="/assets/daisy_ui/controllers/daisy_dropdown_controller-d98bf51b.js">No
DocsUI::chrome changed and the rendered page looks the same, so there are no screenshots. The diff above is the evidence.Test plan
spec/docs_kit/importmap_pin_spec.rb(new): draws the real pin file into anImportmap::Map(importmap-rails 2.2.3, now a dev/test dependency). Covers the default name and asset, no default preload,preload = truepreloading on every entry point,preload = "docs"preloading fordocsonly, andcontrollers_under = "controllers"mapping to the same asset. Three examples were RED before the change.spec/docs_kit/engine_spec.rb: defaults for both options (RED before the change).docs/spec/requests/docs_nav_pin_spec.rb(new): a docs page's import map mapsdocs_kit/controllers/docs_nav_controller, and no modulepreload names it. RED on main: the preload list contained it.docs/spec/system/docs_chrome_spec.rb: docs-nav still connects with eager registration and no preload, and it builds the on-this-page TOC (criterion 4). The example is RED when the eagerdocs_kit/controllersline is commented out.config.docs_kit.controllers_under = "controllers", changedindex.jstolazyLoadControllersFrom("controllers", application)and added a throwaway/plainpage (javascript_importmap_tags+data-controller="hello"). Playwrightperformance.getEntriesByType("resource"):preloaded_module_pathsgrep for docs_nav:[]. All of it was reverted afterwards.config.docs_kit.preload = truein the dogfood app puts/assets/docs_kit/controllers/docs_nav_controller-2c0d75d3.jsback inpreloaded_module_paths.bundle exec rspec: 1181 examples, 0 failures (93.2% line coverage)bundle exec rubocop: no offenses, in the gem and indocs/cd docs && bundle exec rspec: 17 examples, 0 failures (request + Playwright)bundle exec conformance-kit check: passed (locally on Ruby 3.4.2; CI runs 4.0)Deviations & judgment calls
DocsKit::Engine.config.docs_kit, notRails.application.config.docs_kitas the issue sketched. Both are the sameOrderedOptions(Railtie config options are shared; verifiedequal?in the docs app), and the engine's reads in a gem spec where no app is booted."importmap"initializer, sopreload/controllers_undermust be set inconfig/application.rb(likeauto_pin), not inconfig/initializers/.importmap-railsjoins the root Gemfile's development/test group (require: false) so the pin-file spec draws a realImportmap::Map. The root lockfile is untracked; it is not a gemspec dependency, sodocs/Gemfile.lockis unaffected.Importmap::Map#preloaded_module_pathscaches under onecache_keyfor every entry point unless told otherwise;javascript_importmap_module_preload_tagspassescache_key: entry_point. The pin spec does the same, or the entry-point example reads a stale cached result.controllers_under = "controllers"as "through the host's own loader", not "lazy (recommended)". The stock stimulus-railsindex.jsiseagerLoadControllersFrom("controllers", ...), so the setting only gives lazy loading where the host already lazy-loads its own controllers.eagerLoadControllersFrom("docs_kit/controllers", application), which stays supported. On a host that setcontrollers_under = "controllers",--syncwould still inject that line after an eager anchor (dead, harmless) or warn on a lazy-onlyindex.js. Teaching it to readconfig/application.rbis a possible follow-up.controllers_under), Changed (thepreloaddefault) and Fixed (the contradictory registration docs), with an "Upgrading from 1.4" note, rather than one Changed bullet.Summary by cubic
Stops the engine from preloading the docs-nav controller on every page, and collapses three conflicting registration recipes into one documented section.
The auto-pin took importmap-rails' default
preload: true, so every page of a host whose docs share the application's import map carried a 24 KB<link rel="modulepreload">fordocs_nav_controller.js, and a hostpinwithpreload: falsecouldn't override it.New engine options
config.docs_kit.preload(defaultfalse) is the pin'spreload:;truerestores 1.4's behavior, an entry-point name preloads only where that entry point renders.config.docs_kit.controllers_under(default"docs_kit/controllers") lets the host's own loader find the controller by convention; the asset path never changes.Eager-registering sites now fetch the controller one request later on docs pages; set
config.docs_kit.preload = trueto keep the preload. The README no longer recommendslazyLoadControllersFrom("docs_kit/controllers", ...), which logged an autoload error for every host controller identifier.Written for commit 8d70c11. Summary will update on new commits.