Skip to content

dogfood: split ui registry lib/utils.ts so importing cn() doesn't pin a page to the browser #819

Description

@vivek7405

Problem

The cn() class-merger lives in a MIXED module that also holds client-global helpers, so importing even the pure cn() into a page/layout marks that module client-effecting -> browser-shipped (elision can no longer drop it). The moment that page also imports a server-only .server.ts util, it trips no-server-import-in-browser-module and would crash at load. Verified this session: a page importing only cn from #lib/utils/cn.ts is browser-shipped, and adding a .server.ts import makes webjs check fire no-server-import-in-browser-module.

An agent reaching for cn() in a page (a natural thing to do) inadvertently pins the page and then hits the server-import rule. AGENTS.md already documents the shape (#619, "if a util MIXES a pure helper with client-global code (the cn.ts shape), split the client part into its own module") but the framework's OWN ui registry ships it unsplit.

Single source of truth (important for the fix scope)

There is exactly ONE source file: packages/ui/packages/registry/lib/utils.ts (audited: the only non-component .ts in the registry that references client globals; 18 refs). It bundles:

  • PURE helpers: cn(), domId(), ensureId() (no client globals).
  • CLIENT-GLOBAL helpers: Base (HTMLElement / the SSR ServerHTMLElementStub), defineElement (customElements), onBeforeCache (document).

Both consumers copy from this one file:

  • webjs create copies it to the app as lib/utils/cn.ts (packages/cli/lib/create.js ~L88-92, rewriting ../lib/utils.ts -> #lib/utils/cn.ts).
  • webjs ui add <component> copies registry components whose import of ../lib/utils.ts is rewritten to the same #lib/utils/cn.ts.

So fixing the registry source fixes both paths. The audit found NO other cn.ts-like mixed helper (every other client-global reference is inside /components/, which are custom elements and correctly ship).

Design / approach

Step 0, discard dead code first (this shrinks the problem a lot). The ui package used to build components as raw custom elements on an HTMLElement base; it now extends WebComponent from @webjsdev/core. So the HTMLElement-era helpers in lib/utils.ts are LEFTOVER DEAD CODE. Audit confirms:

  • Base (the HTMLElement / ServerHTMLElementStub base) and defineElement (customElements) are used in 0 registry components and are imported nowhere in the ui package (the only other reference is packages/ui/packages/website/lib/utils.ts, which is just another COPY of this file, not a consumer).
  • Base + defineElement + the ~90-line ServerHTMLElementStub class + the HasHTMLElement const are the BULK of the 18 client-global refs (HTMLElement, customElements).

Delete all of that dead code. After removal, the file's only remaining client-global helper is onBeforeCache (used in 6 components), and its document access is typeof document === 'undefined'-GUARDED (SSR-safe). The rest is pure: cn (16 components), domId / ensureId (ensureId in 6; domId is an internal helper of ensureId, exported but referenced by 0 components directly), and the *Class string helpers.

Step 1, then handle what remains. Determine whether the guarded onBeforeCache still makes the module count as client-effecting under the elision analyzer (component-elision.js):

  • If a typeof-guarded document reference does NOT trip the client-effecting verdict, then simply deleting the dead code may already make lib/utils.ts inert, so importing cn no longer pins a page, and no split is needed.
  • If a guarded document reference STILL trips it, split onBeforeCache into a small separate client module (e.g. lib/dom.ts), leaving lib/utils.ts pure (cn + domId + ensureId + *Class). The 6 components that use onBeforeCache import it from the new module.

Do the cheaper outcome the analyzer allows; verify with the counterfactual test below (a page importing cn + a .server.ts must not trip no-server-import-in-browser-module).

Then update the CLI registry-copy logic (create.js + the webjs ui add path) to emit BOTH modules and rewrite BOTH import specifiers, and repoint the registry components' imports (Base/defineElement/onBeforeCache from the client module, cn from the pure module).

Implementation notes (for the implementing agent)

  • Two packages change in lockstep: @webjsdev/ui (the registry source + component imports) and @webjsdev/cli (the copy + import-rewrite in create.js ~L88-92 and the webjs ui add reader). Both need a version bump.
  • Landmine: the scaffolded app groups helpers under lib/utils/ and aliases as #lib/utils/cn.ts (Implement # path-alias imports (package.json imports + resolveImport alias expansion) #555/Convert deep relative imports to #/ aliases across the in-repo apps + examples #556); keep the alias-rewrite correct for the new second module too.
  • Landmine: registry components are copied by webjs ui add into components/ui/; their import of the client helpers must resolve after the split (rewrite target must exist in the scaffolded app).
  • Tests: test/scaffolds/scaffold-ui-integration.test.js + packages/cli/test/* (a scaffolded app + a webjs ui add app must pass webjs check and webjs typecheck); ADD a counterfactual proving a page importing only cn is NO LONGER browser-shipped (does not trip no-server-import-in-browser-module when it also imports a .server.ts). This is the headline behavior.
  • Docs: update the AGENTS.md dogfood: ship a class-name utility (cn) in @webjsdev/core #619 note ("the cn.ts shape") to point at the now-split modules; sync CONVENTIONS.md if it references cn.ts.
  • Not an intellisense change, so no nvim re-vendor.

Acceptance criteria

  • The dead HTMLElement-era helpers (Base, defineElement, ServerHTMLElementStub, HasHTMLElement) are removed from packages/ui/packages/registry/lib/utils.ts (they are used by 0 components after the WebComponent migration). The stale copy in packages/ui/packages/website/lib/utils.ts refreshes when the website re-scaffolds / re-copies the registry util.
  • After the dead-code removal (and a split of onBeforeCache only if the analyzer still flags the guarded document), importing cn carries no client globals.
  • A page/layout importing only cn is NOT marked client-effecting (elision can drop it), proven by a counterfactual test (importing cn + a .server.ts no longer trips no-server-import-in-browser-module).
  • Both webjs create and webjs ui add <component> produce apps that pass webjs check + webjs typecheck with the split modules wired.
  • AGENTS.md dogfood: ship a class-name utility (cn) in @webjsdev/core #619 note updated; ui + cli version bumps + changelogs.

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Type

No type

Fields

No fields configured for issues without a type.

Projects

Status
Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions