-
Notifications
You must be signed in to change notification settings - Fork 54
feat: support custom types from tsr plugins #1585
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release53
Are you sure you want to change the base?
feat: support custom types from tsr plugins #1585
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request updates type constraints for timeline objects and keyframes from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/shared-lib/src/core/model/Timeline.ts (1)
39-39: Critical:DeviceTypeExtdoes not exist intimeline-state-resolver-types.Same issue as in
packages/blueprints-integration/src/timeline.ts. The pipeline failures confirm thatTSR.DeviceTypeExtis not available in the current version oftimeline-state-resolver-types. This PR cannot be merged until the dependency on Sofie-Automation/sofie-timeline-state-resolver#412 is resolved.
🧹 Nitpick comments (1)
packages/documentation/docs/for-developers/device-integrations/tsr-plugins.md (1)
124-124: Optional: Consider simplifying "all of the" to "all the".The phrase "all of the" can be simplified to "all the" for a more concise style.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/blueprints-integration/src/timeline.ts(1 hunks)packages/documentation/docs/for-developers/device-integrations/tsr-plugins.md(1 hunks)packages/shared-lib/src/core/model/Timeline.ts(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: Node CI
packages/shared-lib/src/core/model/Timeline.ts
[error] 39-39: TS2724: '/home/runner/work/sofie-core/sofie-core/packages/node_modules/timeline-state-resolver-types/dist/index' has no exported member named 'DeviceTypeExt'. Did you mean 'DeviceType'?
[error] 62-62: TS2724: '/home/runner/work/sofie-core/sofie-core/packages/node_modules/timeline-state-resolver-types/dist/index' has no exported member named 'DeviceTypeExt'. Did you mean 'DeviceType'?
🪛 LanguageTool
packages/documentation/docs/for-developers/device-integrations/tsr-plugins.md
[style] ~124-~124: Consider removing “of” to be more concise
Context: ... { input: number } } ``` With this, all of the sofie timeline object and tsr types wil...
(ALL_OF_THE)
🪛 markdownlint-cli2 (0.18.1)
packages/documentation/docs/for-developers/device-integrations/tsr-plugins.md
92-92: Hard tabs
Column: 1
(MD010, no-hard-tabs)
93-93: Hard tabs
Column: 1
(MD010, no-hard-tabs)
94-94: Hard tabs
Column: 1
(MD010, no-hard-tabs)
106-106: Hard tabs
Column: 1
(MD010, no-hard-tabs)
112-112: Hard tabs
Column: 1
(MD010, no-hard-tabs)
113-113: Hard tabs
Column: 1
(MD010, no-hard-tabs)
117-117: Hard tabs
Column: 1
(MD010, no-hard-tabs)
118-118: Hard tabs
Column: 1
(MD010, no-hard-tabs)
119-119: Hard tabs
Column: 1
(MD010, no-hard-tabs)
120-120: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🔇 Additional comments (2)
packages/shared-lib/src/core/model/Timeline.ts (1)
61-73: Clarify the purpose and scope of the keyframe-levelabSessionfield.The addition of
abSessiontoTimelineKeyframeCoreExt(lines 69-72) is a distinct feature from the object-levelabSessionsproperty inTimelineObjectCoreExt. WhileabSessionsmanages named sessions at the object level,abSessionenables per-keyframe player selection within an AB pool. Confirm whether this per-keyframe capability is intentional and documented in the PR.Additionally, the reference to a critical
DeviceTypeExtissue on line 62 could not be located in the codebase. Please clarify what specific concern was raised earlier aboutDeviceTypeExt.packages/blueprints-integration/src/timeline.ts (1)
21-21: Verify ifDeviceTypeExtis now available in the currenttimeline-state-resolver-typesversion.The code in both
packages/shared-lib/src/core/model/Timeline.tsandpackages/blueprints-integration/src/timeline.tsusesTSR.DeviceTypeExtin generic type constraints. The project depends ontimeline-state-resolver-types@10.0.0-nightly-release53-20251030-091938-982ec3103.0, a nightly build from October 30, 2025.While the original pipeline error indicated that
DeviceTypeExtwas missing, the current nightly version appears to be recent enough to potentially include the fix from the referenced dependency PR. Confirm whether:
- The dependency PR (Sofie-Automation/sofie-timeline-state-resolver#412) has been merged
- The nightly version currently specified includes the
DeviceTypeExtexport- The project compiles without the TS2724 error
If the issue persists, ensure the dependency PR is merged and update the package version accordingly.
packages/documentation/docs/for-developers/device-integrations/tsr-plugins.md
Show resolved
Hide resolved
836b9a0 to
c9e9f99
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
About the Contributor
This pull request is posted on behalf of Superfly
Type of Contribution
This is a: Code improvement
Depends upon Sofie-Automation/sofie-timeline-state-resolver#412
Current Behavior
When using a TSR plugin, it requires some messy type casting to be able to create timeline objects for the plugin.
This is the sofie-core side of a solution to that problem.
To use it, inside of blueprints you can add a file such as
tsr-extend.d.ts:Blueprints will be happy with these custom types.
In order for this to work, some usages of
DeviceTypehave to be replaced withDeviceTypeExt, so that they accept the custom DeviceTypes.Testing Instructions
Other Information
Status
Summary by CodeRabbit
Documentation
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.