Skip to content

test(compiler): pin that a file edit moves no server-function id - #3121

Closed
frenzzy wants to merge 1 commit into
solidjs:nextfrom
frenzzy:test/compiler-id-stability
Closed

test(compiler): pin that a file edit moves no server-function id#3121
frenzzy wants to merge 1 commit into
solidjs:nextfrom
frenzzy:test/compiler-id-stability

Conversation

@frenzzy

@frenzzy frenzzy commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Server-function ids are the addresses deployed clients already hold, so keying them on identity rather than position (7c551ae, #3109) is a wire contract — and nothing currently guards it.

The fixture suite cannot: every fixture in __tests__/directives/ is a single fixed source with frozen expected output, so it pins the id format and has no second version to compare against. The positional scheme passed an identically shaped suite; the fix regenerated 137 files. Reintroducing position goes green after exactly the regeneration a developer performs reflexively when snapshots fail — and the failure it lets through is the silent one: an old tab calling getUser reaching deleteUser with a 200.

This is the differential half. Compile two versions of one file and require every surviving name to keep its address:

  • a function appended at the end
  • a function inserted at the top
  • the first function deleted
  • the two reordered
  • a body edited

Plus the two consequences worth stating rather than discovering:

Checked by mutation

Reverting create_id to format!("{}-{}", self.hash, self.count - 1) fails 4 of the 7:

expected 'c8cb6025-1' to be 'c8cb6025-0'
expected 'c8cb6025-0' to be 'c8cb6025-1'
expected [ 'c8cb6025-0', 'c8cb6025-1' ] to not include 'c8cb6025-1'
expected 'c8cb6025-0' to match /^handler-[0-9a-f]{1,8}$/

The other three pass under both schemes — inserting at the top, deleting the first function, and editing a body never moved an id even positionally. Worth keeping for exactly that reason: most edits look safe, which is why the bug survived. Deleting the first function is the surprising member of that set, and it is surprising only because the old numbering ran in reverse.

Compiler suite green: 29 files, 4325 passed. Tests only, no changeset.

Raised as a gap in #3120.

Ids are the addresses deployed clients already hold, so keying them on
identity rather than position (7c551ae) is a wire contract. The fixture
suite cannot express it: every fixture is a single fixed source, so it
pins the id FORMAT and has no second version to compare against — the
positional scheme passed an identically shaped suite, and the fix
regenerated 137 files. Reintroducing position would go green after
exactly the regeneration a developer performs when snapshots fail.

This is the differential half: compile two versions of one file — append,
insert, delete, reorder, edit a body — and require every surviving name
to keep its address. Also pins the two consequences worth being explicit
about: a rename is a new address (a clean 404, not another function), and
among functions sharing a descriptive name position still decides.

Checked by reverting create_id to `<hash>-<count>`: 4 of the 7 fail.
@changeset-bot

changeset-bot Bot commented Aug 30, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 6194ee7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codspeed-hq

codspeed-hq Bot commented Aug 30, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 136 untouched benchmarks
⏩ 132 skipped benchmarks1


Comparing frenzzy:test/compiler-id-stability (6194ee7) with next (5230666)

Open in CodSpeed

Footnotes

  1. 132 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@frenzzy

frenzzy commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by d6a4a52 — closing.

Your directives-id-scheme.test.js covers this better than mine did. Both were rebuilt against a create_id reverted to <hash>-<ordinal>:

directives-id-scheme.test.js     6 failed | 1 passed
directives-id-stability.test.js  4 failed | 3 passed

Deriving the ids from an independent implementation catches a change to the hash, the path derivation or the format, where my differential version only notices when an edit happens to move something — and three of my cases pass under both schemes, since inserting at the top, deleting the first function and editing a body never moved an id even positionally. That is a weaker signal for the same regression.

The README line is the part I would not have thought to write, and it is probably the most durable of the three: a fixture regeneration that changes an id is a protocol change, not churn. That is the sentence that stops the next person waving it through.

@frenzzy frenzzy closed this Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant