6.0.0 (1/2): Bun.Image, a structural timezone rule, and deleting render:'spa' - #262
Closed
sebyx07 wants to merge 3 commits into
Closed
6.0.0 (1/2): Bun.Image, a structural timezone rule, and deleting render:'spa'#262sebyx07 wants to merge 3 commits into
sebyx07 wants to merge 3 commits into
Conversation
…d PNG codec
`packages/core/src/image/` was a from-scratch codec — Huffman tables, DCT,
scanline filters, a Lanczos resampler. 1,679 source lines and 1,292 test lines,
deleted. `pipeline.ts:20` gave the reason it existed:
Bun ships no image API and the contract forbids `sharp`, so WebP and AVIF
are probed and served, never synthesised here.
That stopped being true in Bun 1.3.14 — the version already pinned. `Bun.Image`
is statically-linked libjpeg-turbo / libspng / libwebp with SIMD resize kernels.
Still zero dependencies.
WebP encodes now, so `@ultimat3/storage`'s default variant format and its default
`.webp` key extension finally agree: a `srcset` entry can be served, not merely
named.
`Bun.Image.backend` is pinned to `'bun'` on every call. That forces the static
codecs and Highway geometry everywhere, which is what `variantKey`'s
content-addressed cache requires — a platform-dependent variant is a cache that
never hits. The cost is that AVIF and HEIC are refused on every platform rather
than working on two of them, which is one behaviour instead of two.
`blurDataUrl` is `Bun.Image.placeholder()` — a deterministic ThumbHash at most
32px on its long edge. `BLUR_PLACEHOLDER_WIDTH` is deleted; there is no width to
name. ThumbHash quantises the aspect ratio, and `responsiveImage()` paints the
LQIP as `background-size: cover` inside a box sized from the real dimensions, so
nothing shifts.
With no `format`, the SOURCE format is kept. The old rule guessed from the pixels
and turned an opaque PNG into a JPEG; a heuristic that can flatten a logo
eventually does.
`png-pixels.ts` (183 lines) survives as a raw-pixel seam, and writing its test
first found a real bug: unfiltering into a `Uint8ClampedArray` CLAMPS where PNG
filters are mod-256, so `255 + 1` gave 255 instead of 0. Invisible against our
own filter-0 writer, wrong against every adaptive one — it forced alpha to 255,
flattening every transparent PWA icon. `Bun.Image` has no compositor and no
raw-pixel terminal, and the PWA maskable safe zone needs both, which is why the
seam exists at all.
Bun agrees with Pillow byte-for-byte on five reference decodes (RGBA, greyscale,
grey+alpha, 16-bit, palette+tRNS). Determinism is asserted on both the fast and
composite paths, since the content-addressed cache depends on it.
Refs #252
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KgsU1WBJMAjnLaazvfuJmD
BREAKING. Bun 1.4 ships ICU 78, where `new Intl.DateTimeFormat('en',
{timeZone:'CET'})` no longer throws. `resolve()` delegated the IANA-ness
judgement to `Intl`, so a runtime upgrade alone reopened the framework's
"no date without an explicit IANA timeZone" rule — silently, and in the
direction that fails dangerous:
isValidTimeZone('CET') -> true (was false) also EST, EST5EDT, GMT, MST
`Intl` answers "can I format this", never "is this IANA", and at ICU 78 the two
stopped agreeing. The judgement is now structural: `UTC`, or a name containing
`/`. It cannot move with the runtime again.
43 names change answer, enumerated with their replacements in CHANGELOG.md. The
24 geographic `backward` links and the three UTC aliases swap textually — each
verified against seven probe instants including both 2026 DST transitions. The
GMT family maps to `Etc/GMT`, not `UTC`, because `UTC` renders a different zone
label. The eleven abbreviations have no mechanical replacement, and that is the
defect: an abbreviation names no jurisdiction and carries no DST rule.
No denylist. A list of refused abbreviations grows with every tzdata release, and
there is no structural rule that keeps `CET` out and lets `Japan` in.
`X_TIMEZONE_INVALID`'s `cause:` was itself false — `Japan` IS an IANA name, a
`backward` link in shipped tzdata — so it now reads "not an IANA Area/Location
zone name", which is true for all four refused classes. The `fix:` instructs both
classes: the mechanical swap for a legacy link, and "name the city" for an
abbreviation, because only the author can choose one.
Three `format.test.ts` expectations move to anchored patterns rather than pinned
ICU-78 literals, so the suite is green on ICU 75 and 78 both and this does not
have to merge with the Bun upgrade.
Refs #251
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KgsU1WBJMAjnLaazvfuJmD
… worked
BREAKING. Every `spa` route ever declared served an empty document — 200, correct
headers, blank page:
<body><div id="x-root"></div></body>
`dev-render.ts:205` passed `chunks: []`, hardcoded; `prerender.ts` refused the
mode outright; the container mounted the same table. No document in the
framework's history ever carried a <script>. And `renderSpa` never read
`entry.component`, so a `spa` page's own component was dead code in every mode —
along with its `hydrate` and `budget.js`, which nothing could honour.
The mode's own header claimed the shell was "identical for every actor and
therefore cacheable" while it emitted `private, max-age=0, must-revalidate` and
required a `policy`. `MODE_SPECS.spa.prerenderable` was `true` while
`isPrerenderable` is `render === 'static'`.
The deployed demo had already rejected it in writing, one release earlier:
`ssr`, not `spa`: the shell has data to render, and a `spa` shell would ship
that decision to the browser to be asked again.
With `solid-loader.ts` landed, `island({ src })` is a working client model with a
bundler, a content-addressed URL, a boot runtime and a budget measured in real
bytes. `spa` was a second path to interactivity that delivered strictly less. Not
a different thing — a worse island.
`createRouter` goes with it: 236 lines exported from `@ultimat3/render`, 490
lines of tests, zero callers. Its header called it "load-bearing for every app
page"; nothing loaded it. It existed to drive a `spa` shell, and it was built on
a premise `solid-loader.ts` disproves — "inject reactive primitives, import no
solid-js" — because Solid's reactivity is a compile-time contract no injected
runtime can stand in for.
`SPA_ROOT_ID` was never a `spa` symbol: `routeBody` wraps EVERY mode's body in
it. It returns as `ROOT_ELEMENT_ID` in `render-html.ts`, same value.
The guard is a loop over `RENDER_MODES` itself — every mode puts the route
component inside the hydration root — so a fifth mode arriving without a renderer
is a build error, not a blank page.
`@ultimat3/admin`'s generated views join its custom pages at `ssr` +
`hydrate: 'never'`; admin views are pure functions of props with no local state,
so there was never page-level state to hydrate.
Four more hand-copied `RenderMode` unions still admitted 'spa' and typechecked
green after the deletion — `http`, `seo`, `manifest`, `pwa`. All corrected.
`pwa`'s mapped `spa -> cache-first`, the only mode that gave an `app/` route a
shared cache entry. Five copies of one closed set with nothing comparing them is
filed as #261.
Refs #245, #247, #261
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KgsU1WBJMAjnLaazvfuJmD
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 31 minutes Limit details: You’ve used the included review currently available. Your 77 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. You’re in a promotional period — use the checkbox below to run this review for free:
On-demand reviews are free for the next 31 days. After that, they cost $0.25 per reviewed file. How can I continue?Run this review now using the option above, or comment You can also wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (60)
Comment |
Contributor
Author
|
Closing — the split boundary was wrong, and CI proved it in the most direct way possible. This PR deletes So the dependency runs both ways across the cut. There is no clean two-way split at that boundary, and there is no point hunting for one: a PR that cannot build on its own must not be merged to satisfy a file-count guideline. It would leave a commit on Everything here is preserved verbatim — the same three commits are the first three of #263, which now targets
#263 is 161 files, over the ~120 I aim for and just over CodeRabbit's 150 threshold. That is a real cost and I am taking it deliberately, because the alternative is shipping a broken intermediate state. The seven commits are individually scoped and individually reviewable, which is the mitigation available. |
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
1 participant
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.
First of two. Three deletions, each removing something that promised what the code did not do — the sweep 4.0.0 and 5.0.0 ran, applied to what was left.
60 files · net −2,400 lines1.
Bun.Imagereplaces the hand-rolled codec — #252packages/core/src/image/was a from-scratch JPEG and PNG codec: Huffman tables, DCT, scanline filters, a Lanczos resampler. 1,679 source lines and 1,292 test lines, deleted.pipeline.ts:20gave the reason it existed:That stopped being true in Bun 1.3.14 — the version already pinned. Still zero dependencies.
WebP encodes now, so
@ultimat3/storage's default variant format and its default.webpkey extension finally agree — asrcsetentry can be served, not merely named.Bun.Image.backendis pinned to'bun'on every call: byte-identical output across platforms, whichvariantKey's content-addressed cache requires. The cost is AVIF/HEIC refused on every platform rather than working on two — one behaviour instead of two.Writing the replacement's test first found a real bug. Unfiltering into a
Uint8ClampedArrayclamps where PNG filters are mod-256, so255 + 1gave 255 instead of 0. Invisible against our own filter-0 writer, wrong against every adaptive one — it forced alpha to 255, flattening every transparent PWA icon. Unreachable before because the old encoder only emitted filter 0.A 183-line raw-pixel seam survives deliberately:
Bun.Imagehas no compositor and no raw-pixel terminal, and the PWA maskable safe zone needs both.Bun agrees with Pillow byte-for-byte on five reference decodes (RGBA, greyscale, grey+alpha, 16-bit, palette+tRNS).
2. A timezone is
Area/Location, orUTC— #251 · BREAKINGBun 1.4 ships ICU 78, where
new Intl.DateTimeFormat('en',{timeZone:'CET'})no longer throws:A runtime upgrade alone reopened the "no date without an explicit IANA timeZone" rule — silently, in the direction that fails dangerous.
Intlanswers "can I format this", never "is this IANA".The judgement is now structural, so it cannot move with the runtime again. 43 names change answer, tabled in
CHANGELOG.mdwith verified replacements — each checked against seven probe instants including both 2026 DST transitions. The GMT family maps toEtc/GMT, notUTC, becauseUTCrenders a different zone label.No denylist: a list of refused abbreviations grows with every tzdata release, and no structural rule keeps
CETout while lettingJapanin.X_TIMEZONE_INVALID'scause:was itself false —Japanis an IANA name, abackwardlink in shipped tzdata.3.
render: 'spa'andcreateRouterdeleted — #245, #247 · BREAKINGEvery
sparoute ever declared served an empty document — 200, correct headers, blank page:chunks: []was hardcoded,prerender.tsrefused the mode, andrenderSpanever readentry.component— so aspapage's own component was dead code in every mode, along with itshydrateandbudget.js.The mode's header claimed the shell was "cacheable" while it emitted
private, max-age=0, must-revalidate. The deployed demo had already rejected it in writing, one release earlier.createRoutergoes with it: 236 lines exported, 490 lines of tests, zero callers, header calling itself "load-bearing for every app page".The guard is a loop over
RENDER_MODESitself — every mode puts the route component inside the hydration root — so a fifth mode without a renderer is a build error, not a blank page.Four hand-copied
RenderModeunions still admitted'spa'and typechecked green after the deletion. One mappedspa → cache-first, the only mode giving anapp/route a shared cache entry. All corrected; the five-copies problem is filed as #261.Gate
bun run verify— 14 of 19 passed, 5 skipped, 0 failed.Part 2 (#TBD) carries the i18n gate step, the generators, the reference app's first reactive island, and the docs sweep.
🤖 Generated with Claude Code
https://claude.ai/code/session_01KgsU1WBJMAjnLaazvfuJmD
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.