Conversation
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
ryansolid
changed the title
ryansolid
force-pushed
the
feat/text-positions
branch
from
September 30, 2026 14:29
0708b2a to
7a3c1be
Compare
ryansolid
force-pushed
the
examples/hackernews
branch
from
September 30, 2026 14:29
fd35abb to
10fc740
Compare
Size (brotli, eager entry chunk)
Bundled with Rolldown (what Vite ships), brotli q11, decimal KB. Caps in |
Coverage Report for CI Build 36783491364Coverage remained the same at 75.991%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Merging this PR will not alter performance
Comparing 
Footnotes
|
…ng layout
The thread's collapse was a `Toggle` client component wrapping a template
slot. It is now a binding slot on server markup: the recursive `Comment` is
a server component, and each comment with replies calls
`props.toggle({ $key: c.id })` and binds the toggle's `open` class name,
its `onClick`, its label (a text position) and the replies' `display`. The
fill is the SPA twin's `Toggle` without its markup: a signal in the body,
getters over it, a plain handler. No client components remain.
Both twins take the authoring layout: each route file holds its query with
the server function inline (markup in one twin, data in the other), and
`src/server/hn.ts` is a plain module behind `import "server-only"`, with
the boundary types referenced from `vite-env.d.ts` (TypeScript 6 rejects
the undeclared side-effect import). `lib/`, `api.ts`, `views.tsx` and
`components/toggle.tsx` go. `CommentDefinition` gains `id` and
`StoryDefinition.id` is a number, as the data carries.
The READMEs lead with the collapse, name their coordinates, and correct the
bundle check (grep the client JavaScript; `app.css` carries both class
names). A server spec pins the class-array form at a binding position on
both faces.
Co-authored-by: Claude via Cursor <noreply@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The feeds and the user page have no client half, so they become server
routes: `serverRouteComponent(query(...))`, with the router making the
call from the match on navigation and on link hover, so they need no route
component and no `preload`. The feeds are one `/:type?` route filtered to
the five feed names, with `page` read through a hand-written search schema.
The story route keeps its component because of the collapse fill. The nav
leaves the `Loading` boundary, since it does no I/O and the shell can wait
on it. The SPA twin uses the same route pattern and filter, and parses
`?page` itself.
Router 2.0.0-next.29 sent a schema-less server route's args as
`{ params, search: undefined }`. The JSON argument check rejects that, so
client navigation to the user page silently never happened. next.30
fixed it (#615), so every example that uses the router moves to next.31.
Co-authored-by: Claude via Cursor <noreply@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The collapse only hides replies, which is what <details> does natively, so the binding slot was a heavier way to do what the browser already does. Both twins now render the same <details> with a CSS label swap: the server twin's thread has no client code, its story route becomes a server route like the feeds and user page, and the SPA's Toggle component is deleted. The toggle styles move to the <summary> so font sizes don't compound with nesting. Markup stays byte-identical between the twins. Both roots dim the page while a navigation is pending (`useIsRouting()`), with a short delay so a navigation that hover preloading already made fast never flickers. The old page stays up instead of going blank. The plan records the revision, and that the occlusion proof (content a client component doesn't place ships once, as a record) has had no demo or end-to-end test since the early HN demo's deep-collapsed replies were dropped. Co-authored-by: Claude via Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
The notes demo moves to the authoring layout and toward the React original. The shell is a client component, as App.js is; only the sidebar list and the note preview are server components. The server shell had existed only to host the search field as a binding slot, and with the shell on the client the field is a plain client component. The editor's data is a plain query (getNoteEdit) instead of a server component used as a data loader. router.tsx holds the routes, the root preload and getNoteList, and server-config.ts passes that Router to the flight collector, as in the fullstack template. The excerpt now mounts only while expanded, as in the demo, so a collapsed excerpt ships once as an sc:region record and expanding it makes no request. The list is newest first and the excerpt is the rendered markdown as plain text, both matching the demo; the excerpt renders on the server, so marked stays in the lazy editor chunk only. The occlusion specs prove the single-copy claim end to end. The server spec checks each excerpt ships once (markup where placed, a record where not) and that late placement is locked to records. The hydration spec adopts the document with the network stubbed to throw, then expands, collapses and re-expands, with each text on screen once. The README, the grid plan and the principles doc are updated: the notes search field is no longer a binding-slot example, and the hackernews occlusion gap is closed. Co-authored-by: Claude via Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
ryansolid
force-pushed
the
examples/hackernews
branch
from
September 30, 2026 21:34
68ac1c4 to
43e2ade
Compare
ryansolid
changed the title
The README opens with the example's place on the grid: the client-owned, request/response corner beside hackernews-spa, twin of todos-server, and the baseline for the planned board example. It points at todos.ts as the file to read. The todos.ts header keeps its instruction to read the file for its layering rather than as a list of React-to-Solid renames, but drops the rebuke. The error map moves from module scope into createTodos, so each instance has its own; the unused TodoActions type goes; the row checkbox uses onChange like toggle-all. Vite 7 -> 8, as in the other grid examples. The grid plan marks todos built. Co-authored-by: Claude via Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>
brenelz
reviewed
Oct 1, 2026
|
|
||
| // Nothing for the client to fill, so it is a server route like the feeds and | ||
| // the user page. | ||
| export default serverRouteComponent(getStory); |
Contributor
There was a problem hiding this comment.
sometimes the file router injects this right? Or was this changed
serverRouteComponent
brenelz
reviewed
Oct 1, 2026
| @@ -0,0 +1,37 @@ | |||
| /** | |||
| * Copyright (c) Facebook, Inc. and its affiliates. | |||
Contributor
|
So this is the difference betweeen the hackernews and hackernews-spa?
|
This was referenced Oct 1, 2026
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
3 participants
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.
Summary
The idiomatic pass over the examples in
documentation/plans/examples-grid-plan.md, one commit per example. Each example takes the authoring layout and the shape a user would write by hand. A binding slot appears only where a client-owned position sits inside server markup, so the pass also records where they turn out not to fit. Framework bugs found along the way are reproduced in specs and fixed in their own PRs with changesets. This PR stays examples, docs and tests.Rebased onto
nextnow that #3713 and #3714 have merged.So far:
hackernewsandhackernews-spa(three commits), thennotestogether with the end-to-end occlusion specs (one commit), thentodos(one commit). More examples will land here as commits.hackernewsandhackernews-spaBoth twins are in the authoring layout, every
hackernewsscreen is a server route, and the thread has no client code.The collapse is native
<details>. It only hides replies, which is what<details>does, so both twins render the same<details>with a CSS label swap. This PR first built the collapse as a binding slot (a fill per comment binding the class, handler, label anddisplay). Review replaced it: that was a heavier way to do what the browser already does. The SPA'sTogglecomponent is deleted, and the toggle styles move to the<summary>so font sizes don't compound with nesting.Every
hackernewsscreen is a server route:export default serverRouteComponent(getStory). The router makes the call from the match on navigation and on link hover, so there are no route components and nopreloadexports. The feeds are one/:type?route, filtered to the five feed names, withpagecoming from a hand-written search schema. The SPA twin uses the same route pattern and filter, and parses?pageitself. The URLs haven't changed.Navigation dims instead of blanking. Both roots put the routed content in
<div class={["page", { routing: useIsRouting() }]}>. A pending navigation keeps the current page up at reduced opacity until the next one lands. A 150ms delay means a navigation that hover preloading already made fast never flickers.The nav is outside
Loading. It does no I/O, so the shell waits on it at no cost. It renders inline before the content, and navigation leaves it alone.Router →
2.0.0-next.31in every example that uses it (hackernews, hackernews-spa, notes, room).next.29sent a server route with no search schema as{ params, search: undefined }. The JSON argument check rejects that, so client navigation to the user page quietly never happened.next.30fixed it (solidjs/solid-router#615). The lockfile diff touches only the router.Both twins take the authoring layout:
const getStory = query(async ({ params }) => { "use server"; … }, "story")). The thread's file also holds the recursiveComment.src/server/hn.ts(and the capture) is a plain module that beginsimport "server-only". It's byte-identical in both twins.lib/,api.ts,views.tsxand bothcomponents/toggle.tsxfiles are deleted. The SPA keepscomponents/for its templates.routes/story.tsxshows the thesis: the samegetStory, returning markup in one and data in the other.Example-local fixes:
CommentDefinitiongains theidthe data carries.StoryDefinition.idisnumber, which is what the capture and the live API both return.app.csscontains the class names, so the old instruction to grep all ofdist/client/could never come back empty.vite-env.d.tsfiles reference@solidjs/vite-plugin/boundary-modules: TypeScript 6 rejects the undeclaredimport "server-only"(TS2882). The plan records this for later examples.READMEs describe server components, server routes, the native collapse and the navigation dim. Both twins name their coordinate on the grid. The server twin's README no longer implies only the router ships: the runtime that mounts server components ships too, and the server twin's boot JavaScript is larger than the SPA's: 56.1KB against 37.0KB minified and brotli-compressed (63.9KB against 41.5KB gzip), counting the client entry and its static imports. The five lazily loaded chunks are the same size in both twins.
A spec pins the class-array form at a binding position (
class={["toggle", { open: t.open }]}) on both server faces. There was no test for it before. It was written for the first collapse and still covers the runtime independently.notesReact's server-components demo, moved to the authoring layout and toward React's own shape.
The shell is a client component, as React's
App.jsis. Only the sidebar list and the note preview are server components. The server shell had existed only to host the search field as a binding slot. With the shell on the client, the search field is a plain client component, as it is in the demo. This is the pass's first finding about binding slots: one needs a position inside server markup, and here the only such positions are the per-note onesSidebarNoteContentalready owns. The binding-slot demos are nowtodos-serverandchat's copy button.The editor's data is a plain query.
getNoteEditreturns the note. Before, a server component served as a data loader, filling the editor slot with raw text.The excerpt mounts only while expanded, as in the demo. A collapsed excerpt is never placed, so the server sends it once, as an
sc:regionrecord rather than as hidden markup, and expanding it mounts from that record with no request.The layout matches the fullstack template.
router.tsxholds the route table, the root preload andgetNoteList;app.tsxis the shell;server-config.tspasses thatRouterto the single-flight collector. The collector takes a router instance because not every app uses Solid Router, so each router template does this wiring itself.Two differences from the demo, fixed: the list is newest first (the demo's
order by id desc), and the excerpt is the rendered markdown as plain text rather than raw markdown. The excerpt renders on the server, somarkedstays in the lazy editor chunk only.Files:
server/db.ts(waslib/db.ts, nowimport "server-only"), androutes/note.tsxholds the note's queries. Deleted:lib/api.ts,routes.ts,server/App.tsx,server/Note.tsx,server/NoteList.tsx,server/actions.tsandcomponents/searchField.ts. The save and delete actions live inNoteEditor.tsx, which posts them. The README is rewritten around the file map against the React demo.todosThe SPA control, twin of
todos-server. It has no server side, so the authoring layout doesn't apply, and the code was already right.hackernews-spa), its twin, and the plannedboardexample it's the baseline for. It points attodos.tsas the file to read.todos.tsheader keeps its instruction to read the file for its layering rather than as a list of React-to-Solid renames. It exists because agents reading the example missed the layering, but it's no longer phrased as a rebuke.createTodos, so each instance has its own. The unusedTodoActionstype goes, and the row checkbox usesonChangelike toggle-all.todos' entry.Occlusion specs
The single-copy claim for content a client component places conditionally had no end-to-end test. The early HN demo showed it live with deep replies collapsed by default (
b3f48999e), and63cd06688dropped it. The only spec fed the client hand-written records. The new specs close that gap:test/server/frame-occlusion-document.spec.tsx: each excerpt ships once, as markup where placed and as ansc:regionrecord where not. When placement happens late (after an async read), both are records only.test/hydration/frame-occlusion-document.spec.tsx: adopts that document withfetchstubbed to throw, then expands, collapses and re-expands. There are no warnings or requests, and each text is on screen once.Both were shown able to fail: the server spec with placed content hidden by CSS instead of unplaced, and the client spec with the region record stripped from the document.
Docs
hackernews,notesandtodosbuilt, records the review revisions, and closes the occlusion gap.notes' expand toggle as the markup-slot case and record why the search field stopped being an attribute-slot example. Its example map reads "expand slot".Public API changes
None. Examples, docs, tests and example dependency versions only, so no changeset.
How did you test this change?
After the rebase onto
next:pnpm build, then all three@solidjs/websuites (client 1115, server 1385, hydration 271, all passing).tsc --noEmitandvite buildforhackernews,hackernews-spaandnotes.hackernews twins:
tsc --noEmitandvite buildfor both twins.server-onlymarkers, and fails with the plugin's error on a deliberate client import of~/server/hn(reverted).hackernews' client JS contains noitem-view-comments-header,comment-children,news-item,user-view, collapse-label or nav markup. Neither twin's containsnode-hnapior the capture./,/new?page=2, the user page and the 1,406-comment thread (760,997 characters, 652<details>). The server twin's pages carry no binding-slot markers. The earlier route checks (/top,/ask?page=abcfalling back to page 1, and an unknown path rendering no view) passed on the same route table.checkVisibility()reports the replies hidden./, feed → thread, thread → feed, and pagination.{"params":{"id":"lxm"}},{"params":{"type":"new"},"search":{"page":2}}), and the click makes no request.class="page"with no dim in the first render. On a cold click,routingis set within about 8ms, the old page stays up with noLoading...fallback, and the dim clears as the new view lands (394ms leaving the big thread, about 200ms for a user page). A hovered navigation lands in about 20ms without dimming.notes:
tsc --noEmitandvite build. The boot JavaScript is 62.0KB brotli (62.8KB before).markedis only in the lazy editor chunk, anddate-fnsandunstorageare in no client chunk.sc:regionrecord, and nowhere as markup.?searchText=fills the box and filters the list./. On a rename, the sidebar item keeps its node and expanded state and flashes. A new note goes to the top, and the notes below move down with their nodes and expanded state.todos:
tsc --noEmitandvite build.