Uh oh!
There was an error while loading. Please reload this page.
refactor(config): split entrypoints into pure default, /io and /effect (CLI-2231) - #6315
Conversation
…t (CLI-2231) - `.` now exports only the pure, browser-safe surface (schema, types, document encode/decode, defaults/sparse mapping, error classes, constants); io.ts/functions-manifest.ts were physically partitioned (config-document.ts, functions-manifest-model.ts) so no file-IO code enters the pure module graph, enforced by a static import-graph test - `./effect` is the Effect-native superset (services, layers, Effect-returning programs); 22 apps/cli files repointed - `./io` replaces the unused `./bun`/`./node` subpaths with bun/node/browser/default export conditions; the Promise facades now share one lazily-built ManagedRuntime (was rebuilt and leaked per call) scoped to FileSystem+Path only - effect moves to peerDependencies (tag identity), platform packages to optional peers; dedent/smol-toml stay regular dependencies - ADR 0009 entrypoint decision recorded; packages/config/AGENTS.md added
Coly010
left a comment
There was a problem hiding this comment.
Review of the three-entrypoint split. What was verified before commenting: packages/config unit tests (183/183), lint/fmt/knip, and tsc --noEmit clean; apps/clitsc --noEmit clean plus targeted unit/integration runs over the repointed areas; pnpm install --frozen-lockfile clean with a byte-identical lockfile; a scripted scan of all 92 @supabase/config importers confirming zero value-imports of the 14 symbols removed from ., no file mixing . and ./effect, and zero consumers of the removed ./bun/./node subpaths; empirical bun build --target=browser runs against each ./io condition; and the ManagedRuntime lifecycle against the pinned effect source (build fiber memoized, scope finalizers removed via addObserver, no accumulation — the never-disposed singleton is sound).
The layering itself is right and the partition is a verbatim move with no behavior drift. Seven inline comments below: four should-fix (the browser condition of ./io breaks builds instead of producing the documented runtime error; the stdin-leak regression test does not fail when the regression is reintroduced; the purity walker has two reproducible false negatives; ADR 0009 records a factually wrong mechanism for the effect peer-dep move) and three consider (./io is not type-self-sufficient; a new type-level import cycle through errors.ts; the two monorepo-wide import rules are prose-only). All findings were reproduced or source-verified, not pattern-matched.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- io-browser.ts is now side-effect-free: exports the seven facade names as async functions that reject at invocation with a curated message (named imports previously failed at build time and namespace imports crashed isomorphic bundles at module evaluation) - stdin-leak regression guard moved to a dedicated test file where the facade call is genuinely the first (old guard stayed green under a revert to the full BunServices layer); added a structural no-Terminal context assertion - entrypoint-purity walker rebuilt on Bun.Transpiler.scanImports with a hard-fail on non-literal dynamic imports (regex scanner missed backtick specifiers and over-erased past semicolon-less type aliases) - ADR 0009 peer-dependency rationale restated on true grounds (Effect v4 service identity is the string key, not module tag identity) - ./io entrypoint is now self-sufficient (export * from index in all three condition targets, superset test-enforced) - ConfigFormat moved to a zero-import leaf, breaking the type-level errors -> config-document -> project -> errors cycle - new monorepo import-contract test forbids in-repo @supabase/config/io and deep-path imports
…1-refactor-config-package-to-default-export-node-with-effect # Conflicts: # packages/config/package.json # pnpm-lock.yaml
…nitialized CI never hits this (fresh clones leave submodules uninitialized), but any local tree after `pnpm repos:install` fails the new root targets from #6311/#6312: oxlint's nested-config discovery loads .repos/effect/.oxlintrc.json and dies on its uninstalled JS plugin (ignorePatterns does not gate config discovery — this repo is single-root-config by design, so disable nested configs), and knip reports ~1.1k .repos files as unused (submodule contents are invisible to git status but not to knip's project glob).
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@7b23f744f7b1ceed756fad69929ca70df7051368Preview package for commit |
avallete
left a comment
There was a problem hiding this comment.
The three-entrypoint split holds on HEAD: . stays a physical, browser-safe partition; ./effect is a real unshadowed superset; ./io is a lazy Promise facade over FileSystem | Path only, with a side-effect-free browser stub. The seven earlier self-review threads are still fixed. CLI Effect call sites moved to /effect; remaining @supabase/config imports are schema/types/constants.
Two non-blocking nits inline — neither needs to land before merge:
- The monorepo import-contract test will not re-run when another workspace adds a forbidden
/ioor deep import (test:unitinputs are project-local). Add workspaceextraInputsthe waypackages/stackalready does; scanning.tsxtoo would close the other documented hole. - A couple of leftover review-history comments in the facade tests can shrink to the current invariant.
Approve as-is.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
The monorepo import-contract test scans other workspaces' sources, but test:unit inputs were project-local, so nx affected / a warm cache skipped it exactly when a consumer added a forbidden import. Wire workspace-wide extraInputs for packages/config (same hatch packages/stack uses), scan .tsx too, and shrink two review-history comments to the current invariants.
Uh oh!
There was an error while loading. Please reload this page.
Summary
Splits
@supabase/config's public surface into three strictly layered entrypoints, per CLI-2231 (including its 2026-08-24 amendment for browser consumers):.— pure, browser-safe. Schema, types, document encode functions, defaults/sparse mapping, error classes, and constants only. No Effect-returning functions are exported, and no@effect/platform-*,node:, orbun:module can enter its transitive graph — Studio (see feat: Add a config drift page in Studio supabase#48906) can bundle this platform-free. The pure document layer was physically partitioned out ofio.ts/functions-manifest.ts(newconfig-document.ts,functions-manifest-model.ts) so the invariant is structural, and a static import-graph test (entrypoint-purity.unit.test.ts) enforces it: file allowlist, bare-specifier allowlist (effect/smol-toml/dedent), dynamic-import()/requiredetection, and inline-snapshot pinning of both export surfaces. Browser bundle drops 387 KB → ~355 KB with zero file-IO code reachable../effect— Effect-native superset. Re-exports everything in.plus the Effect-native surface (loadProjectConfig,saveProjectConfig, discovery, env loading,inferFunctionsManifest,ProjectConfigStore,projectConfigStoreLayer, …), so Effect-native consumers need one specifier (Option B from the issue). The 22apps/clifiles that import Effect-native symbols were repointed with byte-identical symbol lists; the other 37 consumer files keep the bare specifier unchanged. On the issue's open decision:resolveProjectValue/resolveProjectSubtreestay Effect-typed in./effectonly (the amendment requires.to carry plain-sync signatures; unwrapping can be revisited in CLI-2234)../io— Promise facade for non-Effect consumers, replacing the unused./bun/./nodesubpaths (verified zero consumers) withbun/node/browser/defaultexport conditions. Thebrowsercondition resolves to a side-effect-free stub exporting the same seven names, each rejecting at invocation with a pointer to., instead of dragging@effect/platform-nodeinto a browser bundle (kept side-effect-free so isomorphic/edge bundles that never call config IO still build and load). All three condition targets re-export the pure surface, so./ioconsumers never need a second specifier for parameter/return types. Both facades now share one lazily builtManagedRuntimevia a common factory — previously each call didManagedRuntime.make(...)fresh and never disposed it — and the runtime layer is narrowed toFileSystem + Path(the former full*Services.layerattached a permanentprocess.stdinlistener via Terminal)../schema.json— unchanged.Dependency reshape:
effectmoves topeerDependencies— Effect v4 resolves services by string key, so this isn't about tag identity; the real grounds are a singleeffectversion across the dependency graph while rc-to-rc internals churn, dedupe/bundle size, and letting the consumer own the version — and@effect/platform-bun/-nodebecome optional peers (a consumer only needs the one matching their runtime), each mirrored indevDependenciesfor local build/test.dedentandsmol-tomlstay regular dependencies.Docs: ADR 0009's open "config package architecture" bullet is resolved with the entrypoint contract, and
packages/config/AGENTS.mdrecords the monorepo import rule (Effect symbols →/effect; pure-only →.;./iois for external non-Effect consumers only).Out of scope (tracked separately): npm publishing/compiled dist output (CLI-2232/CLI-2233) and the public-surface audit (CLI-2234).
Linked issue
Linear: CLI-2231
open-for-contributionlabel (or I'm a Supabase maintainer).Checklist
fix(cli): …).pnpm check:allandpnpm testpass for the workspace(s) I touched.