Repository navigation
[2.x] Move casing helpers to @codama/fragments - #1174
Conversation
|
trevor-cortex
left a comment
There was a problem hiding this comment.
Summary
Consolidates the casing helpers into @codama/fragments, aligns the word split with the spec's casing-collision rule (spec#175), removes the *CaseString brands from @codama/node-types, and repoints every in-repo consumer. nodes-from-anchor gets a private anchorCasing.ts so Anchor discriminator preimages are decoupled from Codama's casing policy, and the IIFE build gets an esbuild plugin that stubs fs/path/url so codama's browser bundle builds again.
I read through all 55 files. The casing rewrite itself looks correct: I traced the two regexes against the validator's previous getIdentifierWords (identical split, given identifiers can only contain [A-Za-z0-9_]) and against change-case's default split regexps, which is what Anchor's TS client uses for snakeCase. The new word splitting tests, the pinned initialize discriminator (afaf6d1f…) and the acronym cases give good coverage of the behaviour that actually changed.
Things to watch out for
1. Layering: the core lockstep group now depends on a renderer-support package. @codama/visitors-core, @codama/validators, @codama/visitors and codama now depend on @codama/fragments, whose browser build still carries bare import 'node:fs' / import 'node:path'. The tsup plugin only fixes this repo's IIFE bundle; any downstream browser consumer of visitors-core/validators (webpack 5 errors on unresolved node:fs, Vite warns and stubs) now inherits that problem, when previously none of the core packages touched Node built-ins. The PR lists "remove the bare imports at the source" as a follow-up, but this PR is what propagates them into the core dependency tree, so I'd suggest doing that first (or in this PR) — e.g. gating the node:fs import itself behind __NODEJS__ via a dynamic require/import, or moving the fs helpers to a @codama/fragments/node subpath — so the plugin becomes unnecessary rather than a permanent workaround. Not blocking, but I think it's the main thing to decide before merging.
Relatedly, @codama/fragments sits outside the fixed group in .changeset/config.json. updateInternalDependencies: "patch" will keep the workspace:* pins moving, so this is fine mechanically, but it does mean every fragments release now cascades a patch bump through the entire core group — worth being aware of, or consider whether fragments should join the group now that it's load-bearing for the validator.
2. Import specifier ordering will likely fail pnpm lint. In every file where CamelCaseString was replaced, IdentifierString was placed at the top of the @codama/nodes import list, out of the case-insensitive alphabetical order the rest of the repo follows (e.g. IdentifierString, AccountNode, EventNode in dynamic-parsers/src/parsers.ts; IdentifierString, assertIsNode, … in extractPdasVisitor.ts, unwrapDefinedTypesVisitor.ts, unwrapInstructionArgsDefinedTypesVisitor.ts, unwrapTupleEnumWithSingleStructVisitor.ts, updateAccountsVisitor.ts, and both visitors test files). The PR notes lint was only run on the adapted packages, which is exactly the set that doesn't contain these lists. A repo-wide pnpm lint:fix should sort it out.
3. camelCase now returns plain string. Node factories accept plain strings and brand internally, so most call sites are unaffected. But code that types variables/returns explicitly as IdentifierString and feeds them from camelCase(...) now needs identifierString(...) — extractPdasVisitor.ts (getUniquePdaName return, resolvedName reassignment, usedNames.has(candidate)) is one example. These are in not-yet-adapted packages so they're not new failures per the PR's baseline comparison, but they're the pattern to expect when those packages get adapted.
Smaller notes
packages/nodes/src/shared/identifiers.ts— theidentifierStringdocblock still says uniqueness "is resolved by case-folding".brands.tswas updated to the camelCase-form wording; this one should match.anchorCasing.ts— the docblock attributesgetAnchorStructName's behaviour to Anchor using the Rust struct name "as written", but the tolerance forstakeEntry→StakeEntryactually mirrors the TS client'scamelcase(name, { pascalCase: true, preserveConsecutiveUppercase: true }). Worth citing so the source of truth is traceable (inline comment).tsup.config.base.ts— the plugin applies to every IIFE build, not justcodama's, and silently stubs anyfs/path/urlimport rather than failing. Fine today since only the guarded fragments code hits it, but it removes a useful build-time signal if some future package accidentally imports a Node built-in unguarded. Another reason to fix it at the source.
For subsequent reviewers
- The casing output change (
getURL→getUrl,HTTPServer→http_server,UPPERCASED→Uppercased) is a behavioural change for anything that consumedcamelCase/snakeCase/pascalCasefrom@codama/nodesorcodama, not just renderers. The changesets PR needs to call it out clearly. - I did not fetch spec#175 to verify the word split against the spec text verbatim; I verified it against the validator's previous implementation (which presumably was written from the spec) and against
change-case. Someone with the spec open should confirm the digit rule (foo1Bar→foo1|Bar,foo1barone word) matches. - Downstream renderers (
renderers-js,renderers-rust, …) that still import casing from@codama/nodeswill break on this; make sure the migration order accounts for it.
288055e to
1e37115
Compare
trevor-cortex
left a comment
There was a problem hiding this comment.
Re-review
Re-fetched all 58 files. Every point from my previous pass has been addressed, and the fixes are the right ones rather than workarounds:
- Layering — the tsup IIFE stub plugin is gone, replaced by a dedicated
@codama/fragments/casingsubpath. I checkedcasing.ts: it has zero imports, so consumers ofvisitors-core/validators/codamano longer transitively touchnode:fs/node:pathin any build target. The subpath is wired consistently with the existing./javascript/./rustones (exportswith react-native/browser/node conditions,browserfield remaps,typesVersions,files, tsupENTRY, and the agadoo tree-shake loop).@codama/fragmentsis now in the changesetsfixedgroup. - Import ordering —
IdentifierStringis correctly placed in every@codama/nodesimport list now, and@codama/fragments/casingimports sort before@codama/nodesas expected. extractPdasVisitor.ts—camelCase(...)results feedingIdentifierStringslots are now wrapped inidentifierString(...).- Docblocks —
nodes/src/shared/identifiers.tsnow matches the camelCase-form wording inbrands.ts, andanchorCasing.tscites the TS client'scamelcase(name, { pascalCase: true, preserveConsecutiveUppercase: true })as the source ofgetAnchorStructName's behaviour.
Nothing new to flag. The codama re-export is an explicit named list rather than export *, which is a reasonable choice — it just means any future helper added to @codama/fragments/casing needs a one-line follow-up in packages/library/src/index.ts if it should be reachable from codama.
For subsequent reviewers
- Still worth someone with spec#175 open confirming the digit rule (
foo1Bar→foo1|Bar,foo1barone word) verbatim; I verified against the validator's previous implementation andchange-case, not the spec text. - Renderers still importing casing from
@codama/nodeswill break on this; the migration order needs to account for it, and the final changesets PR must call out both the casing output change and the removed@codama/nodes/@codama/node-typesexports.
cf343be to
8a167b4
Compare
4354390 to
fc393d4
Compare
b91b06a to
2e1217c
Compare
fc393d4 to
42578fd
Compare
2e1217c to
6a242eb
Compare
7d45051 to
be3fca0
Compare
6a242eb to
2e57b9c
Compare
be3fca0 to
26fcebe
Compare

This PR makes
@codama/fragmentsthe single home of the casing helpers — exposed through a new dependency-free@codama/fragments/casingsubpath — aligns them with the word split of the spec's casing-collision rule (codama-idl/spec#175), and removes the casing brands.Why
spec#175 requires renderers to derive every casing of an identifier from the spec's word split, so that identifiers the validator accepts never collide in generated code. Two near-identical copies of the casing helpers existed — in
@codama/nodesand@codama/fragments— and both split words before every capital, which disagrees with the spec on acronyms (getURL→get_u_r_l,HTTPServer→h_t_t_p_server). The casing brands (CamelCaseString,SnakeCaseString, …) no longer describe any v2 node attribute either: identifiers areIdentifierString, and v2 mandates no casing, so the brands promised something the model no longer uses.Changes
@codama/fragments—titleCase, which every other casing helper builds on, now splits words at any non-alphanumeric run (dropping empty words), between a lowercase letter or digit and an uppercase letter, and between an uppercase letter and an uppercase letter followed by a lowercase letter. The UPPER_SNAKE special case is gone, as the split now handles it. Outputs change only for runs of capitals:camelCase('getURL')getURLgetUrlsnakeCase('HTTPServer')h_t_t_p_serverhttp_serverpascalCase('UPPERCASED')UPPERCASEDUppercasedsnakeCase('FROM UPPERCASED TITLE CASE')f_r_o_m_u_p_p_e_r_c_a_s_e_d_…from_uppercased_title_case@codama/fragments/casing— a new subpath exporting only the casing helpers. The main@codama/fragmentsentry also exposes the Node-only filesystem helpers, whose browser build keeps barefs/pathimports; the subpath has no imports at all, so browser bundles of the non-rendering packages (including thecodamaIIFE bundle) never reach them.@codama/nodes— removescamelCase,capitalize,kebabCase,pascalCase,snakeCaseandtitleCase. Import them from@codama/fragments/casinginstead.@codama/node-types— removesCamelCaseString,KebabCaseString,PascalCaseString,SnakeCaseStringandTitleCaseString, and updates theIdentifierStringdocblock to the casing-collision rule. Nothing generated referenced them; regenerating produces no change.codama— re-exports the casing helpers from@codama/fragments/casing, soimport { camelCase } from 'codama'keeps working..changeset/config.json— adds@codama/fragmentsto thefixedgroup, as the core packages now depend on it at runtime.Consumers (
visitors-core,visitors,validators,nodes-from-anchor,dynamic-codecs,dynamic-parsers) — depend on@codama/fragments/casingfor casing, and useIdentifierStringwhere they usedCamelCaseString.validatorsdrops its private word-split helpers in favour ofcamelCase.nodes-from-anchor— Anchor discriminators hash Anchor's own name conversions, which must not follow Codama's casing: instructions hash the snake_case name (setHTTPConfig→global:set_http_config), while accounts and events hash the Rust struct name as written, acronyms preserved (HTTPConfig→account:HTTPConfig). A privateanchorCasing.tsnow provides both, so the new casing split does not change any hash. Tests pin the existing known values, Anchor'sinitializediscriminator, and acronym cases.Verification
fragments,node-types,nodes,visitors-coreandvalidatorstype-check, build, tree-shake, lint and pass their tests. The packages not yet adapted to v2 were compared against the base branch: no new type errors and no new test failures invisitors,nodes-from-anchor,dynamic-codecs,dynamic-parsers,dynamic-client,dynamic-address-resolutionanddynamic-instructions, withnodes-from-anchorgaining the new discriminator tests. Thedynamic-*packages also load far more of their test files than on the base branch, which failed to importcodamaat runtime; every test passing on the base branch still passes.Still failing on the base branch, unrelated to this PR: the
codamaIIFE bundle (v1 node imports in the not-yet-adapted@codama/visitors, unchanged here), and 27 stale@codama/spec-generatorstests that still assert v1 shapes.Follow-ups
@codama/fragments/casingand drop the casing brands when adapted to v2.@codama/fragmentsbrowser build still contains bareimport "fs"/import "path", which only matters to renderers bundling it for the browser; worth removing at the source.@codama/fragmentscasing output change and the removed@codama/nodes/@codama/node-typesexports.