feat(js): public user-type accessors on SailsProgram and SailsService#1343
Merged
Conversation
Closes #1320. Adds `SailsProgram.programTypes` and `SailsService.types`, both `ReadonlyMap<string, Type>`, so consumers can enumerate user types declared in the v2 IDL without reaching into the private `_doc` field via `as any`. `SailsService.types` is declared-only — it returns the service's own `types {…}` block. For the merged extends-chain scope, `program.resolveInService(name, decl)` and `service.extends[base].types` already cover the use cases. Both maps are empty when the corresponding IDL block is absent. Also re-exports the v2 IDL AST typings (`Type`, `TypeDecl`, `ITypeStruct`, `ITypeEnum`, `ITypeAlias`, `IIdlDoc`, `IServiceUnit`, etc.) from the top-level `sails-js` entry via `export type * from 'sails-js-types'`, so consumers can `import type { Type } from 'sails-js'` without the `sails-js/types` subpath. The internal `sails-js-types` workspace package stays `private: true`. The new maps are built eagerly in the constructors. They are typed `ReadonlyMap` to block `.set()` at compile time; runtime mutation is not enforced. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces public ReadonlyMap accessors for user types in SailsProgram and SailsService, and re-exports IDL AST types from the package root. A review comment suggests adopting a lazy indexing strategy for these type maps to improve performance and maintainability by avoiding unnecessary initialization overhead in the constructor.
The new `SailsService.types` getter is reachable on the class but was missing from the `ISailsService` read-surface interface. Surfaced by /simplify code-quality review as a leaky abstraction (the interface lists `functions`, `queries`, `events`, `extends`, `routeIdx` — `types` belongs there too). Two-line interface addition. No runtime change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…package
The previous form, `export type * from 'sails-js-types'`, emitted an
unresolvable external module reference into the published `lib/index.d.ts`:
`sails-js-types` is `private: true` and only a devDependency of `sails-js`,
so downstream TypeScript consumers could fail to resolve the root
declaration even for normal value imports like `import { SailsProgram }
from 'sails-js'`.
Re-export from `./types.js` instead. That path resolves at consumer
install time to the rollup-`dts`-bundled `lib/types.d.ts`, which already
inlines every `sails-js-types` interface as a self-contained declaration.
No external module reference, no devDependency leak.
Caught by `codex review` as [P1].
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-type-accessors-1320
vobradovich
approved these changes
May 8, 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
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.
Closes #1320.
Summary
API additions (sails-js v2)
SailsProgram.programTypes: ReadonlyMap<string, Type>for program-level user types declared in the IDL'sprogram {…}block.SailsService.types: ReadonlyMap<string, Type>for the service's owntypes {…}block (declared-only — does not include types inherited viaextends).Type,TypeDecl,ITypeStruct,ITypeEnum,ITypeAlias,IIdlDoc,IServiceUnit, …) from the top-levelsails-jsentry, so consumers canimport type { Type } from 'sails-js'without thesails-js/typessubpath.Both maps are eager-built in the constructor and return an empty Map when the corresponding IDL block is absent. They are typed
ReadonlyMapto block.set()at the type level; runtime mutation is not enforced.For the merged extends-chain scope, consumers continue to use
program.resolveInService(name, decl)or walkservice.extends[base].types. The newservice.typesaccessor intentionally does not surface inherited types — making each service's declared surface explicit and keeping the existing resolution semantics untouched.Why
vara-wallet and other downstream consumers were reaching into
SailsProgram._docviaas anyto enumerate user types (see #1320 for the full writeup). That escape hatch breaks silently the moment a private field is renamed between betas, and consumers couldn't stay type-safe at the AST level becausesails-js-typeswas only reachable through thesails-js/typessubpath.Test Coverage
10 new tests in
js/test/idl-v2-types-accessors.test.tscovering every code path on the new surface:Tests: 14 → 15 (+1 new file, 10 new test cases). 84 of 84 v2 tests pass on the merged state (10 new + 74 adjacent regression-coverage suites:
idl-v2-parser-type-resolver,idl-v2-type-resolver,event-type-shape,route-idx-header-validation,partial-idl).Pre-Landing Review
Five review passes ran before landing. All findings resolved.
/plan-eng-review/review(gstack pre-landing)sails-js-typesresolution)/code-review-graph review-pr/simplify(3 parallel reviewers)ISailsServiceinterface missingtypesmemberd47b2e95./codex reviewexport type * from 'sails-js-types'ships unresolvable external module refc4696f41by re-exporting from the bundled./types.jsinstead. Issue #1344 updated to reflect 1/4 of the historical leaks closed; 3 pre-existing references remain inlib/sails-idl-v2.d.ts,lib/util.d.ts,lib/parser.d.tsand will be addressed by the build-side fix tracked in #1344.Bot comment from
gemini-code-assist[bot](eager-vs-lazy Map build) replied with the F2 rationale; no code change needed (review thread r3201667845).Plan Completion
SailsProgram.programTypesgetterjs/src/sails-idl-v2.ts:259SailsService.typesgetterjs/src/sails-idl-v2.ts:502typestoISailsServiceinterfacejs/src/sails-idl-v2.ts:25export type *from sails-jsjs/src/index.ts:12(via./types.js)idl-v2-types-accessors.test.ts## Unreleasedentriesservice.typesprogramTypesCOMPLETION: 8/8 DONE. No PARTIAL, no NOT DONE.
Documentation
js/CHANGELOG.mdupdated under## Unreleasedwith two bullets covering the new accessors and the top-level type re-export (including the resolution path via bundled./types.js). No README sync needed — the existing README documents only thesails-js/typessubpath and that path still works exactly as before; the top-level alias is purely additive.No version bump:
js/package.jsonstays at0.5.1since this is part of the next unreleased.Test plan
pnpm test test/idl-v2-types-accessors.test.ts— 10/10 passpnpm testfor adjacent v2 suites — 74/74 pass (no regressions)pnpm build— clean;lib/index.d.tsemitsexport type * from './types.js';lib/types.d.tsis fullydts-bundled (241 lines, all 49 AST types inlined, zero external module references)import type { Type, ITypeStruct } from 'sails-js'typechecks at consumer install time withoutsails-js-typesin node_modulessails-js-typesexternal ref inlib/index.d.ts) — fixed in this PRlib/sails-idl-v2.d.ts,lib/util.d.ts,lib/parser.d.ts— out of scope for this PR, tracked in sails-js: published .d.ts files reference private sails-js-types module #1344🤖 Generated with Claude Code