Support generic record types across the data + grid layer (#3691)#4458
Draft
amcclain wants to merge 17 commits into
Draft
Support generic record types across the data + grid layer (#3691)#4458amcclain wants to merge 17 commits into
amcclain wants to merge 17 commits into
Conversation
Also update RoleModel.ts data casts (as unknown as HoistRole) to preserve
zero-error baseline - these casts worked when data was plain PlainObject/any
but now data is typed as PlainObject & {id: StoreRecordId} which TypeScript's
overlap check rejects. The double-cast is a bridge until Task 8 migrates
RoleModel to Store<HoistRole>.
Fixture lives in data/__typetests__/ and is picked up by the existing tsconfig include pattern (./**/*). Since tsconfig uses emitDeclarationOnly, no runtime artifact is produced. No tsconfig exclude addition needed. Added eslint-disable for no-unused-vars since all variables exist solely for compile-time type assertions.
…mpatibility
Change StoreRecord.data and committedData from T & {id: Id} to T & {id?: Id}.
The required id extension caused PlainObject & {id: StoreRecordId} for untyped
stores, which TypeScript's TS2352 overlap check rejected when casting to concrete
interfaces (e.g. `record.data as HoistRole`). Making id optional eliminates the
concrete named property conflict while preserving full structural covariance.
Also exports RecordData<T, Id> = T & {id?: Id} as a utility type alias, adds a
zero-breakage cast guard to the fixture, and reverts four as-unknown-as-HoistRole
workarounds in RoleModel back to as-HoistRole.
Note: the prescribed conditional-type RecordData (string extends keyof T) was
investigated but caused TypeScript to treat T as invariant in class bodies,
producing 20 errors in Store.ts/StoreSelectionModel.ts. The optional-id approach
achieves the same goal without touching those files.
Per design decision: data reflects exactly the declared field shape, consistent with Hoist's 'id is not a field' model. record.id carries the typed Id; data.id is available only when the data interface itself declares it. Keeps Store<T> covariantly assignable to Store and leaves untyped-store casts working unchanged.
…ta casts - Add local `RoleGridRow extends HoistRole` to cover synthetic fields (`isGroupRow`, `effectiveUserNames`, etc.) added by `processRawData` and `processRolesForTreeGrid` - Type `gridModel` property and `createGridModel()` as `GridModel<RoleGridRow>` - Remove `as HoistRole` cast in `selectedRole` getter - now typed via generic - Remove `as HoistRole` cast in `onRowDoubleClicked` - `record` is `any` there - Retain casts in action callbacks (ActionFnData.record is untyped StoreRecord)
- CHANGELOG.md: New Features entry for opt-in Store/StoreRecord/GridModel/etc. generic <T, Id> params
- data/README.md: Add "Typing Records" subsection under StoreRecord covering interface definition,
id derivation (auto vs. explicit), getValues() return type, opt-in behavior, and current limitation
on column renderer/RecordAction callbacks
- data/StoreRecord.ts: JSDoc note on `data` property clarifying id is stamped at runtime but use
record.id for type-safe access
- data/__typetests__/dataGenerics.typetest.ts: Add getValues() type assertion (returns T & {id: Id})
- Add a 'Typed Records' section to cmp/grid/README.md showing GridModel<T> typed selectedRecord/store.records/record.id, with the renderer/RecordAction untyped-callback caveat and a cross-link to the data package's Typing Records section. - Add a forward pointer to Typing Records from the StoreRecord intro in data/README.md. - Add 'generics'/'typed records'/'RecordId' discoverability keywords to the data and grid doc-registry entries.
The release-prep merge from develop renamed the prior SNAPSHOT to released 86.2.0 and opened 87.0.0-SNAPSHOT; relocate the generics New Features bullet to the new topmost unreleased heading.
Member
Author
|
Companion Toolbox PR exercising these generic data-layer types in the example apps: xh/toolbox#874 |
Use descriptive, T-prefixed type-parameter names across the data + grid generics (Store, StoreRecord, StoreSelectionModel, GridModel, DataViewModel, ZoneGridModel, UrlStore, RestStore, RestGridModel, and their *Config interfaces + the RecordId helper) instead of the ambiguous bare <T, Id>. TData reads clearly as the record's data shape and aligns with ag-Grid's own TData convention at the column-callback boundary.
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.
Closes #3691.
Adds opt-in generic type parameters
<T, Id>across the data and grid layer, so apps can declare a record's data shape once and get typed access wherever records surface.What you get
new Store<Person>({...})/new GridModel<Person>({...})→ typedstore.records[i].data,gridModel.selectedRecord.data, andrecord.id.record.idis typedId(auto-derived from the interface'sidviaRecordId<T>, or pass it explicitly:Store<Trade, string>), so ids flow into typed service APIs without casts.Store,StoreRecord,StoreSelectionModel,GridModel,DataViewModel,ZoneGridModel,UrlStore,RestStore,RestGridModel.Design
T = PlainObject,Id = RecordId<T>), so existing untyped code compiles and behaves exactly as before.record.datais typed as the bare field shapeT- consistent with Hoist's "id is not a field" model.idis the top-levelrecord.id, not a member ofdata.Tappears in output positions only (the write API staysPlainObject), keepingStore<Person>covariantly assignable to plainStore- so all internal framework consumers compile unchanged.RecordSet, validators) stays monomorphic; the public read getters cast at the boundary.Demos
RoleModel(GridModel<RoleGridRow>, dropsrecord.data as HoistRolecasts on the selection path) andClusterObjectsModel(GridModel<ClusterObjectRecord>).Not in scope (deliberate)
RecordActioncallbacks still receive an untypedStoreRecord- typed callback context is a possible future follow-on. The demo migrations honestly retain casts there.Verification
data/__typetests__/dataGenerics.typetest.ts) guards typed data, derived/explicit id, the zero-breakage cast, and covariance.tsc --noEmitandyarn lintclean; docs (data + grid READMEs, CHANGELOG, doc registry) updated.