From 1c0a8769667d5c72bed8d3deecc81d361199f09f Mon Sep 17 00:00:00 2001 From: Matt Drees Date: Thu, 30 Jul 2026 14:43:38 -0600 Subject: [PATCH] fix(cable_install): pin firebase, and stop re-adding firestore.rules Both found pre-flighting cru-bot's 0.5.0 upgrade (#12). The generator registered a controller whose `import ... from "firebase/app"` had nothing to resolve against: those three pins live in the app's importmap, and 0.5.0 removed the vendoring step that used to bring them along. Following the README from scratch produced a module that throws on load. The generator now writes them (still app-side, so the app owns the SDK version), and the README lists them for apps that don't use importmap. firestore.rules is now opt-in (--rules). The cru-terraform solid-gcp module renders and releases that ruleset -- its template is this file modulo ${collection} -- so a copy in the app is a second source of truth nobody deploys, and 0.5.0 made re-running the generator the supported upgrade path, which kept re-adding the file apps had deleted on purpose. Also drops the claim that preload: false makes the firebase modules load only on pages with a stream element. index.js imports the controller at boot, so they load with it; preload: false only keeps them out of the modulepreload set. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 13 +++ docs/DESIGN.md | 7 +- dummy/config/importmap.rb | 7 +- gem/README.md | 31 +++++-- .../cable_install/cable_install_generator.rb | 57 ++++++++++++- gem/test/cable_install_generator_test.rb | 82 ++++++++++++++++++- 6 files changed, 180 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4811162..abac2d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,19 @@ tag.) ## [Unreleased] +- `cable_install` pins the three Firebase modules (`firebase/app`, `firebase/auth`, + `firebase/firestore`) in the app's `config/importmap.rb`. Registering the + controller without them leaves an install whose imports don't resolve — the + module throws on load. The pins go in the app, not the engine, so the app still + owns the Firebase SDK version. Also documented in the README for apps that don't + use importmap. Nobody hit this before 0.5.0 because every installer vendored the + controller and picked the pins up alongside it. +- `cable_install` no longer writes `firestore.rules` unless asked (`--rules`). The + cru-terraform `solid-gcp` module renders and releases that ruleset, so a copy in + the app is a second source of truth nobody deploys — and since 0.5.0 makes + re-running the generator the supported upgrade path, it kept re-adding a file + such apps had deliberately deleted. + ## [0.5.0] - 2026-07-30 - Cable: the Stimulus controller is served by the engine (importmap pin) instead diff --git a/docs/DESIGN.md b/docs/DESIGN.md index f8d3bde..044b362 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -251,7 +251,7 @@ app/controllers/solid_gcp/cable_tokens_controller.rb app/helpers/solid_gcp/cable_helper.rb app/javascript/solid_gcp_cable_controller.js # the Stimulus controller (served, not copied) config/importmap.rb # pins it for host apps (engine importmap) -lib/generators/solid_gcp/cable_install/... # registers the controller + firestore.rules +lib/generators/solid_gcp/cable_install/... # registers the controller, pins firebase ``` ## Configuration (`SolidGcp.config.cable.*`) @@ -348,9 +348,10 @@ streams per request (422) — a page should never need more. Response `debug`, which browsers hide) — silence indistinguishable from success is what let a stale client ship broken for a release. - Host app owns the `firebase` JS dep (`firebase/app`, `firebase/auth`, - `firebase/firestore` — full, not `lite`; lite lacks `onSnapshot`). + `firebase/firestore` — full, not `lite`; lite lacks `onSnapshot`), pinned in the + app's own importmap by `cable_install` so the app controls the SDK version. -## Firestore security rules (template shipped; terraform deploys) +## Firestore security rules (terraform deploys; gem template is opt-in reference) ``` rules_version = '2'; diff --git a/dummy/config/importmap.rb b/dummy/config/importmap.rb index ec33162..5ee0c42 100644 --- a/dummy/config/importmap.rb +++ b/dummy/config/importmap.rb @@ -6,9 +6,10 @@ pin "@hotwired/stimulus-loading", to: "stimulus-loading.js" pin_all_from "app/javascript/controllers", under: "controllers" -# Firebase ESM builds from the gstatic CDN, consumed by the SolidGcp::Cable -# Stimulus controller. Self-contained modules; lazy (preload: false) since the -# cable client only loads them when a stream element is on the page. +# Firebase ESM builds from the gstatic CDN, imported by the SolidGcp::Cable +# Stimulus controller. Self-contained modules; preload: false keeps them out of +# the modulepreload set (they load with the controller). Bump the version freely +# — the app owns the Firebase SDK version, not the gem. pin "firebase/app", to: "https://www.gstatic.com/firebasejs/12.0.0/firebase-app.js", preload: false pin "firebase/auth", to: "https://www.gstatic.com/firebasejs/12.0.0/firebase-auth.js", preload: false pin "firebase/firestore", to: "https://www.gstatic.com/firebasejs/12.0.0/firebase-firestore.js", preload: false diff --git a/gem/README.md b/gem/README.md index 856322f..6890b37 100644 --- a/gem/README.md +++ b/gem/README.md @@ -141,12 +141,31 @@ for a morph refresh. Install the client side with: bin/rails generate solid_gcp:cable_install ``` -That registers the Stimulus controller in `app/javascript/controllers/index.js` and -copies a `firestore.rules` starter. The controller itself is **served by the engine** -(importmap pin) — it is deliberately not copied into your app, because a copy goes -stale: apps that vendored it kept subscribing to the `(default)` database long after -the gem learned about named ones. If you bundle JS with esbuild/webpack and need the -file locally, `--vendor` copies the same file the engine serves. +That does two things: registers the Stimulus controller in +`app/javascript/controllers/index.js`, and pins the three Firebase modules the +controller imports in `config/importmap.rb`: + +```ruby +pin "firebase/app", to: "https://www.gstatic.com/firebasejs/12.0.0/firebase-app.js", preload: false +pin "firebase/auth", to: "https://www.gstatic.com/firebasejs/12.0.0/firebase-auth.js", preload: false +pin "firebase/firestore", to: "https://www.gstatic.com/firebasejs/12.0.0/firebase-firestore.js", preload: false +``` + +Those pins live in your app, not the engine, so you own the Firebase SDK version — +bump it freely. (Without them the registered controller's imports don't resolve and +the module throws on load, so add them by hand if you don't use importmap.) + +The controller itself is **served by the engine** (importmap pin) — deliberately not +copied into your app, because a copy goes stale: apps that vendored it kept +subscribing to the `(default)` database long after the gem learned about named ones. +If you bundle JS with esbuild/webpack and need the file locally, `--vendor` copies the +same file the engine serves. + +Security rules are **not** written by default: the +[cru-terraform solid-gcp module](https://github.com/CruGlobal/cru-terraform-modules/tree/main/applications/solid-gcp) +renders and releases the ruleset, so a copy in the app is a second source of truth +nobody deploys. `--rules` writes a `firestore.rules` starter for apps that deploy rules +themselves. Then, in a view: diff --git a/gem/lib/generators/solid_gcp/cable_install/cable_install_generator.rb b/gem/lib/generators/solid_gcp/cable_install/cable_install_generator.rb index 4e3ae08..d4ea364 100644 --- a/gem/lib/generators/solid_gcp/cable_install/cable_install_generator.rb +++ b/gem/lib/generators/solid_gcp/cable_install/cable_install_generator.rb @@ -18,12 +18,23 @@ class CableInstallGenerator < Rails::Generators::Base CONTROLLER_FILE = "solid_gcp_cable_controller.js" CONTROLLERS_INDEX = "app/javascript/controllers/index.js" CONTROLLER_IMPORT = 'import SolidGcpCableController from "solid_gcp_cable_controller"' + IMPORTMAP = "config/importmap.rb" - desc "Registers the SolidGcp cable Stimulus controller and copies firestore.rules." + # The controller imports these three; they are pinned in the app rather than + # in the engine so the app owns the Firebase SDK version. This is the version + # the gem's own suite runs against — bump it in your importmap when you like. + FIREBASE_VERSION = "12.0.0" + FIREBASE_MODULES = %w[app auth firestore].freeze + + desc "Wires up the SolidGcp cable Stimulus controller (registration + firebase pins)." class_option :vendor, type: :boolean, default: false, desc: "Copy the controller into app/javascript/controllers instead of importing it from the gem" + class_option :rules, type: :boolean, default: false, + desc: "Write a firestore.rules starter (skip it when terraform owns the ruleset, as the " \ + "cru-terraform solid-gcp module does)" + def self.source_paths # The engine's app/javascript is a source path so --vendor copies the # shipped controller rather than a fork of it kept under templates/. @@ -57,16 +68,60 @@ def register_controller JS end + # Without these the registered controller's `import ... from "firebase/app"` + # doesn't resolve and the module throws on load — a registration alone is not + # a working install. + def pin_firebase + unless File.exist?(importmap_path) + say_status :skip, "#{IMPORTMAP} not found — pin the firebase modules however you " \ + "bundle JS (firebase/app, firebase/auth, firebase/firestore)", :yellow + return + end + + if File.read(importmap_path).include?('pin "firebase/app"') + say_status :identical, IMPORTMAP, :blue + return + end + + append_to_file IMPORTMAP, <<~RUBY + + # Firebase ESM builds from the gstatic CDN, imported by the SolidGcp::Cable + # Stimulus controller. Self-contained modules; preload: false keeps them out of + # the modulepreload set (they load with the controller). Bump the version freely + # — the app owns the Firebase SDK version, not the gem. + #{FIREBASE_MODULES.map { |mod| pin_line(mod) }.join("\n")} + RUBY + end + + # Opt-in: for apps on the cru-terraform solid-gcp module, terraform renders and + # releases this ruleset, so a repo-root copy is a second source of truth nobody + # deploys — and re-running this generator (the supported upgrade path) used to + # keep re-adding the file such apps had deliberately deleted. def copy_rules + unless options[:rules] + say_status :skip, "firestore.rules — terraform renders the ruleset; pass --rules for " \ + "a starter if your app deploys it itself", :blue + return + end + copy_file "firestore.rules", "firestore.rules" end private + def pin_line(mod) + %(pin "firebase/#{mod}", to: "https://www.gstatic.com/firebasejs/) + + %(#{FIREBASE_VERSION}/firebase-#{mod}.js", preload: false) + end + def index_path File.join(destination_root, CONTROLLERS_INDEX) end + def importmap_path + File.join(destination_root, IMPORTMAP) + end + def copy_controller copy_file CONTROLLER_FILE, "app/javascript/controllers/#{CONTROLLER_FILE}" end diff --git a/gem/test/cable_install_generator_test.rb b/gem/test/cable_install_generator_test.rb index 22afa9c..718498e 100644 --- a/gem/test/cable_install_generator_test.rb +++ b/gem/test/cable_install_generator_test.rb @@ -17,11 +17,16 @@ class CableInstallGeneratorTest < Rails::Generators::TestCase CANONICAL = SolidGcp::Engine.root.join("app/javascript/solid_gcp_cable_controller.js") INDEX = "app/javascript/controllers/index.js" + IMPORTMAP = "config/importmap.rb" DEFAULT_INDEX = <<~JS import { application } from "controllers/application" import { eagerLoadControllersFrom } from "@hotwired/stimulus-loading" eagerLoadControllersFrom("controllers", application) JS + DEFAULT_IMPORTMAP = <<~RUBY + pin "application" + pin_all_from "app/javascript/controllers", under: "controllers" + RUBY test "registers the engine-served controller instead of copying one" do write_index @@ -33,7 +38,68 @@ class CableInstallGeneratorTest < Rails::Generators::TestCase assert_match(/application\.register\("solid-gcp-cable", SolidGcpCableController\)/, content) end assert_no_file "app/javascript/controllers/solid_gcp_cable_controller.js" - assert_file "firestore.rules" + end + + # A registered controller whose `import ... from "firebase/app"` doesn't resolve + # throws on load, so pinning the three modules is part of installing, not a doc + # footnote. Pinned in the app (not the engine) so the app owns the SDK version. + test "pins the firebase modules the controller imports" do + write_index + write_importmap + + run_generator + + assert_file IMPORTMAP do |content| + %w[app auth firestore].each do |mod| + assert_match( + %r{^pin "firebase/#{mod}", to: "https://www\.gstatic\.com/firebasejs/\d+\.\d+\.\d+/firebase-#{mod}\.js", preload: false$}, + content + ) + end + end + end + + test "re-running does not pin firebase twice" do + write_index + write_importmap + + run_generator + run_generator + + assert_equal 1, File.read(File.join(destination_root, IMPORTMAP)).scan(/pin "firebase\/app"/).length + end + + test "without an importmap it says so rather than failing" do + write_index + + output = run_generator + + assert_match(/#{Regexp.escape(IMPORTMAP)} not found/, output) + assert_file INDEX do |content| + assert_match(/application\.register/, content) + end + end + + # For apps on the cru-terraform solid-gcp module, terraform renders and releases + # the ruleset; re-running the generator used to keep re-adding a file those apps + # deleted on purpose. + test "no firestore.rules unless asked" do + write_index + + output = run_generator + + assert_no_file "firestore.rules" + assert_match(/terraform renders the ruleset/, output) + end + + test "--rules writes the starter" do + write_index + + run_generator [ "--rules" ] + + assert_file "firestore.rules" do |content| + assert_match(/solid_gcp_streams/, content) + end end test "re-running does not register the controller twice" do @@ -63,14 +129,22 @@ class CableInstallGeneratorTest < Rails::Generators::TestCase output = run_generator assert_match(/#{Regexp.escape(INDEX)} not found/, output) - assert_file "firestore.rules" + assert_no_file "app/javascript/controllers/solid_gcp_cable_controller.js" end private def write_index - path = File.join(destination_root, INDEX) + write_destination_file(INDEX, DEFAULT_INDEX) + end + + def write_importmap + write_destination_file(IMPORTMAP, DEFAULT_IMPORTMAP) + end + + def write_destination_file(relative, content) + path = File.join(destination_root, relative) FileUtils.mkdir_p(File.dirname(path)) - File.write(path, DEFAULT_INDEX) + File.write(path, content) end end