fix: rework generated types to minimise variation in type union#430
Conversation
WalkthroughThis change refactors the DeviceOptionsBase generic type system from a single type parameter to two type parameters (TType extending DeviceType and TOptions), and updates code generation to expose a discriminated Type property in generated device type interfaces. The changes propagate this generic signature update across conductor, device, container, and connection management classes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
nytamin
left a comment
There was a problem hiding this comment.
I can confirm this solves an issue for me as well!



About the Contributor
This pull request is posted on behalf of the BBC
Type of Contribution
This is a: Bug fix
Current Behavior
This was a weird one, we appear to be hitting a limit on the size of a type union that appears as soon as one more device type is added:
Deleting any of the existing integration types makes the error go away.
Updating typescript to 5.7 did not resolve it.
Seems to be a designed limitation in typescript: microsoft/TypeScript#40803
Not 100% sure what this change does that avoids that, but it avoids the issue
New Behavior
Instead this reworks the types a little bit, which makes the error go away. Other than a few changes in generics, functionally nothing is really different.
We are not using this Base type in sofie-core, so this should have minimal impact outside of TSR.
Testing Instructions
Other Information
Status
Problem
TypeScript compilation was hitting a type union complexity limit where adding one more device type triggered a cascade of compilation errors. These errors surfaced in
ConnectionManager.tswith multiple TS2345 type assignment incompatibilities. Deleting existing device types would eliminate the errors, and upgrading to TypeScript 5.7 did not resolve the issue. The root cause was excessive variation in howDeviceOptionsBasetype unions were being formed across the codebase.Solution
Reworked generated type definitions to minimize variation in type unions by making the generic type parameterization more explicit and consistent throughout the codebase.
Key Changes
Type Definition Changes (
packages/timeline-state-resolver-types/src/device.ts)DeviceOptionsBasefrom a single generic parameter (T) to two explicit parameters:TType extends DeviceTypeandTOptionstype: DeviceType→type: TTypeoptions?: T→options?: TOptionsGenerated Type Schema (
packages/timeline-state-resolver-tools/bin/schema-types.mjs)Typefield to generated per-directoryDeviceTypesinterfacesDeviceOptionsdeclarations from interfaces to type aliases using the newDeviceOptionsBase<DeviceType.{DeviceTypeId}, {Dir}Options>formDeviceTypeto generated output filesType Propagation Updates
device.ts: UpdatedDeviceandDeviceWithStategeneric constraints to includeType: DeviceTypein theDeviceTypesconstraint and pass bothDeviceTypes['Type']andDeviceTypes['Options']toDeviceOptionsBaseconductor.ts: Updated internal function signatures to useDeviceOptionsBase<any, any>consistently across_mapAllConnections, device mapping, and filtering operationsdeviceContainer.ts: UpdatedDeviceContainerand its staticcreatemethod to use the two-parameterDeviceOptionsBase<DeviceType, unknown>ConnectionManager.ts: Standardized all public event signatures and method return types to useDeviceOptionsBase<any, any>remoteDeviceInstance.ts: UpdatedBaseRemoteDeviceIntegrationandRemoteDeviceInstancegeneric constraints to useDeviceOptionsBase<any, any>Impact
DeviceOptionsBasebase type changes are isolated within TSR and should have minimal impact on sofie-core, which does not directly depend on this base typeConnectionManager.tsby reducing union type complexity