Repository navigation
fix: make a deployed Ultimate app render, and stop answering a browser with JSON #52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| // The app document's global layer, and the only stylesheet in this app that emits top-level CSS: | ||
| // @ultimat3/ui's custom properties (`:root{--color-*;--space-*;…}`) and then its reset. Every rule | ||
| // a component module emits reads those properties through `var(--…)`, so without this file the | ||
| // browser drops every one of those declarations and the app renders unstyled. | ||
| // | ||
| // Exactly one file, imported for its side effect by `global.ts` — never `@use`d from a | ||
| // `*.module.scss`. Each module is a separate Sass compilation, so a `@use` that emits would | ||
| // duplicate the whole `:root` block once per module that reached for it. `shared/tokens.scss` is | ||
| // the authoring layer and stays emit-free for that reason. | ||
|
|
||
| @use '@ultimat3/ui/global.scss'; | ||
| @use './tokens' as t; | ||
|
|
||
| // The two element rules the reset leaves to the app, both product decisions rather than | ||
| // normalization: code sits on a tinted ground, and a scrollable region keeps its scrollbar in the | ||
| // theme's own colours instead of the browser's. | ||
| :where(code, pre) { | ||
| background: t.role('bg-soft'); | ||
| border-radius: t.radius('sm'); | ||
| } | ||
|
|
||
| :where(html) { | ||
| scrollbar-color: t.role('line') t.role('bg'); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| // The regression the deployed app shipped for its whole life: 64kB of component CSS in the | ||
| // document and not one `:root` custom property, so every `rgb(var(--color-bg)/1)` the UI kit | ||
| // emits was dropped by the browser. The page looked structurally correct and rendered unstyled. | ||
|
|
||
| import { stylesFor } from '@ultimat3/render'; | ||
| import { expect, unitTest } from '@ultimat3/testing'; | ||
|
|
||
| /** | ||
| * `await import`, not a static one, for the same reason `loadApp` uses a dynamic import: Bun loads | ||
| * a file's whole static graph before it evaluates the module that installs the `.scss` plugin, so | ||
| * a static `import './global'` here would resolve the stylesheet with no loader in place. The | ||
| * module cache makes repeated calls free, so each test states its own precondition. | ||
| */ | ||
| const loadGlobalLayer = (): Promise<unknown> => import('./global'); | ||
|
|
||
| const occurrences = (haystack: string, needle: string): number => haystack.split(needle).length - 1; | ||
|
|
||
| unitTest('both surfaces carry the token layer, because shared/ is in both graphs', async () => { | ||
| await loadGlobalLayer(); | ||
| for (const surface of ['site', 'app'] as const) { | ||
| expect(stylesFor(surface)).toContain('--color-fg:'); | ||
| } | ||
| }); | ||
|
|
||
| unitTest( | ||
| 'the global layer leads the document, so the reset cannot lose a specificity tie', | ||
| async () => { | ||
| await loadGlobalLayer(); | ||
| expect(stylesFor('site').startsWith(':root{')).toBe(true); | ||
| }, | ||
| ); | ||
|
|
||
| unitTest( | ||
| 'it is emitted exactly once — `--space-8` is defined in one scope and only one', | ||
| async () => { | ||
| await loadGlobalLayer(); | ||
| // Not `--color-fg`: the theme defines that one four times on purpose (light `:root`, the dark | ||
| // media query, and `[data-theme]` for each), so only a theme-invariant token counts copies. | ||
| expect(occurrences(stylesFor('site'), '--space-8:')).toBe(1); | ||
| expect(occurrences(stylesFor('app'), '--space-8:')).toBe(1); | ||
| }, | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| // The one edge that puts the global stylesheet in this app's module graph. `shared/` is loaded by | ||
| // both surfaces and by the framework's own boot scan, so the tokens reach every document without a | ||
| // page having to remember to import them — and `x verify` fails with X_STYLES_GLOBAL_MISSING if | ||
| // this edge is ever cut. | ||
|
|
||
| import './global.scss'; |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,17 +1,77 @@ | ||||||
| @use '../shared/tokens' as t; | ||||||
|
|
||||||
| .hero { | ||||||
| .page { | ||||||
| display: grid; | ||||||
| gap: t.space(4); | ||||||
| padding: t.space(8); | ||||||
| gap: t.space(10); | ||||||
| padding: t.space(8) t.space(6); | ||||||
| background: t.role('bg'); | ||||||
| color: t.role('fg'); | ||||||
| } | ||||||
|
|
||||||
| .hero { | ||||||
| display: grid; | ||||||
| gap: t.space(4); | ||||||
| max-width: 62ch; | ||||||
|
|
||||||
| h1 { | ||||||
| // Wrapping a headline at the last two words reads as a mistake; `balance` evens the lines. | ||||||
| text-wrap: balance; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| .lede { | ||||||
| color: t.role('fg-muted'); | ||||||
| font-size: t.text('lg'); | ||||||
| } | ||||||
|
|
||||||
| .actions { | ||||||
| display: flex; | ||||||
| flex-wrap: wrap; | ||||||
| gap: t.space(3); | ||||||
| } | ||||||
|
|
||||||
| .cta { | ||||||
| justify-self: start; | ||||||
| text-decoration: none; | ||||||
| padding: t.space(2) t.space(4); | ||||||
| border-radius: t.radius('md'); | ||||||
| background: t.role('accent'); | ||||||
| color: t.role('accent-fg'); | ||||||
| } | ||||||
|
|
||||||
| .secondary { | ||||||
| text-decoration: none; | ||||||
| padding: t.space(2) t.space(4); | ||||||
| border: 1px solid t.role('line'); | ||||||
| border-radius: t.radius('md'); | ||||||
| color: t.role('fg'); | ||||||
| } | ||||||
|
|
||||||
| .note { | ||||||
| color: t.role('fg-muted'); | ||||||
| font-size: t.text('sm'); | ||||||
| } | ||||||
|
|
||||||
| .list { | ||||||
| display: grid; | ||||||
| grid-template-columns: repeat(auto-fit, minmax(18rem, 1fr)); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Allow proof cards to shrink on narrow viewports.
Proposed fix- grid-template-columns: repeat(auto-fit, minmax(18rem, 1fr));
+ grid-template-columns: repeat(auto-fit, minmax(min(18rem, 100%), 1fr));📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| gap: t.space(4); | ||||||
| margin-block-start: t.space(4); | ||||||
| } | ||||||
|
|
||||||
| .proof { | ||||||
| display: grid; | ||||||
| gap: t.space(2); | ||||||
| padding: t.space(4); | ||||||
| border: 1px solid t.role('line'); | ||||||
| border-radius: t.radius('lg'); | ||||||
| background: t.role('surface'); | ||||||
|
|
||||||
| h3 { | ||||||
| text-wrap: balance; | ||||||
| } | ||||||
|
|
||||||
| p { | ||||||
| color: t.role('fg-muted'); | ||||||
| font-size: t.text('sm'); | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,46 @@ | ||
| import { expect, unitTest } from '@ultimat3/testing'; | ||
| import { config } from './page'; | ||
|
|
||
| unitTest('the landing page ships zero JS and declares metadata', async () => { | ||
| expect(config.render).toBe('static'); | ||
| expect(config.hydrate).toBe('never'); | ||
| expect(config.budget.js).toBe('0kb'); | ||
| const meta = await config.meta({}); | ||
| // The landing page: what it declares, and what it actually emits. | ||
| // | ||
| // The page module is imported DYNAMICALLY, for the reason `apps/admin/app/admin/page.test.ts` | ||
| // spells out: Bun loads a file's static imports before executing any of them, so a static | ||
| // `import './page'` compiles before `@ultimat3/render`'s module scope installs the `.tsx` loader, | ||
| // and every element comes out as `React.createElement` against a global that does not exist. | ||
|
|
||
| import { beforeAll, expect, test } from 'bun:test'; | ||
| // Side-effect import: `defineCatalogs()` runs on the way through. Without it every `t()` in the | ||
| // tree renders `⟦key⟧` — which is exactly what the last assertion here is checking for. | ||
| import '@social-media-clone/i18n'; | ||
| import { t } from '@ultimat3/i18n'; | ||
| import { renderComponent } from '@ultimat3/render'; | ||
|
|
||
| const FILE = 'apps/web/site/page.tsx'; | ||
|
|
||
| let page: typeof import('./page'); | ||
|
|
||
| beforeAll(async () => { | ||
| page = await import('./page'); | ||
| }); | ||
|
|
||
| test('unit · the landing page ships zero JS and declares metadata', async () => { | ||
| expect(page.config.render).toBe('static'); | ||
| expect(page.config.hydrate).toBe('never'); | ||
| expect(page.config.budget.js).toBe('0kb'); | ||
| const meta = await page.config.meta({ data: {}, params: {}, url: 'https://demo.test/', t }); | ||
| expect(meta.title ?? '').not.toBe(''); | ||
| }); | ||
|
|
||
| // This page shipped for weeks as the generator's placeholder — "Everything you need, one command | ||
| // from shippable" over a single link to `/dashboard`, which 401s a signed-out visitor. A title | ||
| // assertion alone passed the whole time, because the placeholder had a title too. | ||
| test('unit · it says what this deployment is, and links somewhere anonymous', async () => { | ||
| const html = await renderComponent( | ||
| page.HomePage, | ||
| { data: {}, params: {}, url: '/', query: {} }, | ||
| FILE, | ||
| ); | ||
| expect(html).toContain('href="/feed"'); | ||
| expect(html).toContain('href="/signin"'); | ||
| expect(html.toLowerCase()).toContain('seeded'); | ||
| // A missing catalog key renders ⟦key⟧, which is visible on the page and passes every other | ||
| // assertion here — so the absence of one is its own check. | ||
| expect(html).not.toContain('⟦'); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,7 @@ | ||
| // The landing page. site/ is 0kb JS: static render, hydrate never, no framework script tag. | ||
| // It says what this deployment IS — a framework stress test with seeded data — because an | ||
| // unlabelled demo reads as a real network, and the first thing anyone does is try to sign up. | ||
|
Comment on lines
+2
to
+3
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value Make this a module header. Move the header before imports. State the landing page's single responsibility and why it stays static. These lines describe deployment content instead. As per coding guidelines: “Add a 1–4 line header comment to each file stating its single responsibility.” 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
| import { t } from '@ultimat3/i18n'; | ||
| import { defineRoute } from '@ultimat3/render'; | ||
| import styles from './page.module.scss'; | ||
|
|
@@ -14,14 +17,37 @@ export const config = defineRoute({ | |
| }), | ||
| }); | ||
|
|
||
| /** Each row is one framework claim this deployment is the evidence for. */ | ||
| const PROOFS = ['policy', 'realtime', 'offline', 'admin'] as const; | ||
|
|
||
| export function HomePage() { | ||
| return ( | ||
| <main class={styles.hero}> | ||
| <h1>{t('site.home.title')}</h1> | ||
| <p>{t('site.home.description')}</p> | ||
| <a class={styles.cta} href="/dashboard"> | ||
| {t('site.home.cta')} | ||
| </a> | ||
| <main class={styles.page}> | ||
| <section class={styles.hero}> | ||
| <h1>{t('site.home.title')}</h1> | ||
| <p class={styles.lede}>{t('site.home.description')}</p> | ||
| <nav class={styles.actions}> | ||
| <a class={styles.cta} href="/feed"> | ||
| {t('site.home.cta')} | ||
| </a> | ||
| <a class={styles.secondary} href="/signin"> | ||
| {t('site.home.signin')} | ||
| </a> | ||
| </nav> | ||
| <p class={styles.note}>{t('site.home.seeded')}</p> | ||
| </section> | ||
|
|
||
| <section class={styles.proofs}> | ||
| <h2>{t('site.home.proofs.title')}</h2> | ||
| <ul class={styles.list}> | ||
| {PROOFS.map((proof) => ( | ||
| <li class={styles.proof}> | ||
| <h3>{t(`site.home.proofs.${proof}.title`)}</h3> | ||
| <p>{t(`site.home.proofs.${proof}.body`)}</p> | ||
| </li> | ||
| ))} | ||
| </ul> | ||
| </section> | ||
| </main> | ||
| ); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove empty
//comments.Stylelint reports
scss/comment-no-emptyat every listed site. Remove the empty separator so stylesheet lint can pass.dummy/social-media-clone/apps/web/shared/global.scss#L5-L5: Remove the empty//separator.dummy/social-media-clone/apps/web/shared/tokens.scss#L5-L5: Remove the empty//separator.examples/dummy/apps/web/shared/global.scss#L5-L5: Remove the empty//separator.🧰 Tools
🪛 Stylelint (17.14.0)
[error] 5-5: Unexpected empty comment (scss/comment-no-empty)
(scss/comment-no-empty)
📍 Affects 3 files
dummy/social-media-clone/apps/web/shared/global.scss#L5-L5(this comment)dummy/social-media-clone/apps/web/shared/tokens.scss#L5-L5examples/dummy/apps/web/shared/global.scss#L5-L5🤖 Prompt for AI Agents
Source: Linters/SAST tools