Finish the config seam: supabase, turnstile and Google Drive - #25
Open
playforge-coding wants to merge 1 commit into
Open
Finish the config seam: supabase, turnstile and Google Drive#25playforge-coding wants to merge 1 commit into
playforge-coding wants to merge 1 commit into
Conversation
The last three modules reading import.meta.env at module scope. main.jsx is now the only place in the app that touches it. supabase.js was the awkward one: it built its client at module scope, so a straight substitution would have constructed it from an unconfigured seam (ES imports are hoisted, so configureCore runs after every module is evaluated). It now builds inside getSupabase(), memoised — the SDK owns the session and its refresh timer, so there must be exactly one. STORAGE_KEY became storageKey() for the same reason. Only auth.jsx consumed any of this, so the blast radius was one file. supabaseEnabled and googleDriveEnabled go the way of the four flags in the previous PR: both were module-scope Boolean(env), both would have read false. They are hasSupabase() and hasGoogleDrive() on the config. All three land in the browser tier rather than the neutral one — localStorage, window.open for the OAuth popup, and the Turnstile widget are all browser things. The lint rule places them correctly without being told. collab.js keeps its React hook and stays in apps/web, but reads apiUrl() now too, so no module outside main.jsx reaches for the bundler. @supabase/supabase-js and docx move to core's dependencies; nothing in apps/web imports either directly any more. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
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.
Stacked on #24. The last three modules reading
import.meta.envat module scope.main.jsxis now the only place in the app that touches it.supabase.jswas the awkward oneIt built its client at module scope:
A straight substitution would have constructed it from an unconfigured seam — ES imports are hoisted, so
configureCoreruns after every module in the graph is evaluated. It now builds insidegetSupabase(), memoised, because the SDK owns the session and its auto-refresh timer and there must be exactly one.STORAGE_KEYbecamestorageKey()for the same reason.Blast radius turned out to be one file:
auth.jsxwas the only consumer.Two more module-scope flags
supabaseEnabledandgoogleDriveEnabledgo the way of the four in #24 — both were module-scopeBoolean(env), both would have readfalseunder a lazy config. NowhasSupabase()andhasGoogleDrive().All three are browser tier
localStoragefor the session,window.openfor the OAuth popup, the Turnstile widget — the lint rule places them correctly without being told.Also
collab.jskeeps its React hook and stays inapps/web, but readsapiUrl()now, so nothing outsidemain.jsxreaches for the bundler.@supabase/supabase-jsanddocxmove to core's dependencies;apps/webimports neither directly any more.supabaseConfig()needs both halves.Where phase 1 actually stands
I said this would leave
apps/web/src/libcontaining nothing but React. It doesn't, quite — 11 React-free files remain:i18n.js,languages.jsreact-i18nextgit/engine.js,git/load.jsBufferpolyfill — bundler concerns, must stay in the apputils.jscn(), the shadcn/Tailwind helper — UI layerrichText.js(DOMPurify pass),wikimedia.js(browser adapter),presence.js,summarizer.js,git/fs.js,git/sync.jsThat last group is a part C. Nothing blocks it now that the seam is complete — it just wasn't in scope here.
Verification
pnpm lint,pnpm test(core 41, api 27, mcp 14),pnpm build,pnpm build:docsall pass. Verified by grep that noimport.meta.envsurvives outsidemain.jsx, and that no config accessor is called at module scope in either package.