Removed TS rule from DashViewSpec that allowed any type of additional values to be added to a DashViewSpec. #3949
Conversation
… values to be added to a DashViewSpec. + Added renderMode & refreshMode to DashViewSpec.
There was a problem hiding this comment.
Pull Request Overview
This PR enforces stricter type safety in DashViewSpec by removing the TS index signature that allowed arbitrary additional properties and introduces explicit renderMode and refreshMode properties.
- Removed the catch-all index signature from DashViewSpec.
- Added renderMode and refreshMode with appropriate types.
- Updated the CHANGELOG to document these changes.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| CHANGELOG.md | Updated changelog entry to reflect the removal of an overly permissive TS rule |
| desktop/cmp/dash/DashViewSpec.ts | Removed the index signature and added explicit properties renderMode and refreshMode |
| @@ -53,6 +53,15 @@ export interface DashViewSpec { | |||
| /** True (default) to allow renaming the view. */ | |||
| allowRename?: boolean; | |||
|
|
|||
| /** Additional properties to store on the DashView */ | |||
| [x: string]: any; | |||
| /** | |||
There was a problem hiding this comment.
I’m pretty sure renderMode and refreshMode are specific to DashContainerViews (and already exist in the DashContainerViewSpec interface which extends this one):
| @@ -3,6 +3,10 @@ | |||
| ## v73.0.0-SNAPSHOT - unreleased | |||
|
|
|||
| * Support for reporting Client Version in Admin WebSockets tab. | |||
| * ⚠️ Removed TS rule from DashViewSpec that allowed any type of additional values to be added to a DashViewSpec. | |||
| The rule defeats the goal of the DashViewSpec interface, which is to help guide app developers | |||
There was a problem hiding this comment.
TMI, I think. Ok to just say "Removed Incorrect TypeScript constraint..." and let the chips fall where they may
|
@ghsolomon and I discussed this more and decided to try to turn this into a chance to use |
I discussed the existence of
on
DashViewSpecwith @ghsolomon, and he suggested seeing what happens if that was removed. Nothing bad happened in toolbox, but it did reveal that renderMode & refreshMode could be made more "official" on the spec.The presence of
means that misspelling a key like "groupname" would not be revealed by TS.
Hoist P/R Checklist
Pull request authors: Review and check off the below. Items that do not apply can also be
checked off to indicate they have been considered. If unclear if a step is relevant, please leave
unchecked and note in comments.
developbranch as of last change.breaking-changelabel + CHANGELOG if so.If your change is still a WIP, please use the "Create draft pull request" option in the split
button below to indicate it is not ready yet for a final review.