Update WebGui using new types and API functions#216
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the WebGui codebase to the newer openspace-api-js generated types and updated topic APIs, removing a large set of locally maintained type definitions and aligning Redux middleware/subscriptions with the new client library.
Changes:
- Replace many local types/enums (
AnyProperty,EngineMode, events, time, etc.) withopenspace-api-js/generatedequivalents. - Update topic subscription/iteration patterns (e.g.,
for await (const data of topic)andtopic.next()usage). - Remove legacy/local type files and the large
openspace-api-js.d.tsshim.
Reviewed changes
Copilot reviewed 96 out of 97 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/util/propertyTreeHooks.ts | Switch featured node type to PropertyOwnerRedux. |
| src/util/propertyTreeHelpers.ts | Use generated AnyProperty and updated owner types. |
| src/util/fileIOhooks.ts | Rename LogLevel usage to NotificationLevel. |
| src/util/eventBus.ts | Migrate event typing to generated EventData shape (event). |
| src/types/types.ts | Replace local Action/Keybind/AnyProperty with generated types; rename owner type. |
| src/types/Property/propertyGroups.ts | Export generic vector/matrix union types. |
| src/types/Property/property.ts | Export reusable VectorValues/MatrixValues/NumericValues helper types. |
| src/types/openspace-api-js.d.ts | Remove large legacy OpenSpace API declaration file. |
| src/types/enums.ts | Replace LogLevel enum with NotificationLevel; remove local EngineMode. |
| src/redux/version/versionSlice.ts | Use generated SemanticVersion. |
| src/redux/version/versionMiddleware.ts | Update version topic read to topic.next(). |
| src/redux/time/util.ts | Add TimeData derived from generated TimeTopic. |
| src/redux/time/types.ts | Remove local OpenSpaceTimeState type file. |
| src/redux/time/timeSlice.ts | Update updateTime payload type to generated TimeData. |
| src/redux/time/timeMiddleware.ts | Migrate Topic import and iteration; use typed Topic<'time'>. |
| src/redux/skybrowser/types.ts | Remove local SkyBrowserBrowser type file. |
| src/redux/skybrowser/skybrowserSlice.ts | Use generated SkyBrowser and SkyBrowserTopic['data']. |
| src/redux/skybrowser/skybrowserMiddleware.ts | Migrate Topic import/typing; fix stop subscription event spelling. |
| src/redux/sessionrecording/sessionRecordingSlice.ts | Use generated session recording topic/state types and add null-safety. |
| src/redux/sessionrecording/sessionRecordingMiddleware.ts | Migrate Topic import/typing and async iteration; remove unused refresh thunk. |
| src/redux/propertytree/propertyTreeMiddleware.ts | Use generated property/propertyOwner types; adapt property update stream shape. |
| src/redux/propertytree/propertySlice.ts | Switch AnyProperty import to generated type. |
| src/redux/propertytree/propertyOwnerSlice.ts | Rename owner type to PropertyOwnerRedux throughout adapter/slice. |
| src/redux/profile/profileSlice.ts | Base profile state on generated ProfileTopic['data']. |
| src/redux/profile/profileMiddleware.ts | Update profile topic read and log level enum rename. |
| src/redux/missions/missionsSlice.ts | Use generated mission types and convert nested dates to UTC. |
| src/redux/missions/missionsMiddleware.ts | Update missions topic read to topic.next() and handle empty mission set. |
| src/redux/logging/loggingSlice.ts | Rename stored level type to NotificationLevel. |
| src/redux/logging/loggingMiddleware.ts | Use generated LogLevel/LogMessage; map log levels to notifications; update topic typing. |
| src/redux/local/localMiddleware.ts | Switch AnyProperty import to generated type. |
| src/redux/groups/util.ts | Update group computation to PropertyOwnerRedux. |
| src/redux/groups/groupsSliceMiddleware.ts | Update tag collection to PropertyOwnerRedux. |
| src/redux/flightcontroller/flightControllerMiddleware.ts | Use generated flight controller command types; type the topic. |
| src/redux/events/types.ts | Remove local event union type file. |
| src/redux/events/eventsMiddleware.ts | Use typed Topic<'event'> and updated generated event field names. |
| src/redux/enginemode/engineModeSlice.ts | Use generated EngineMode enum. |
| src/redux/enginemode/engineModeMiddleware.ts | Migrate engine mode subscription to typed topic/iteration. |
| src/redux/downloadevent/types.ts | Remove local download event types. |
| src/redux/downloadevent/downloadEventMiddleware.tsx | Use generated DownloadType and typed topic iteration. |
| src/redux/documentation/types.ts | Remove local documentation metadata type. |
| src/redux/documentation/documentationSlice.ts | Use generated LicenseEntry for documentation metadata. |
| src/redux/documentation/documentationMiddleware.ts | Remove cast; rely on generated documentation typing. |
| src/redux/camerapath/cameraPathMiddleware.ts | Migrate camera path subscription iteration to new topic API. |
| src/redux/camera/cameraMiddleware.ts | Migrate camera subscription to typed topic and direct async iteration. |
| src/redux/actions/actionsSlice.ts | Use generated Action/Keybind types. |
| src/redux/actions/actionsMiddleware.ts | Use topic.next() and add type-guarding for responses. |
| src/panels/TimePanel/TimePanel.tsx | Import generated EngineMode; keep local TimeStatus. |
| src/panels/TimePanel/TimeInput/TimeInput.tsx | Rename notification level enum usage. |
| src/panels/TimePanel/DeltaTimeStepControl.tsx | Import generated EngineMode. |
| src/panels/SessionRecordingPanel/types.ts | Use generated playback data state and remove local recording enum. |
| src/panels/SessionRecordingPanel/SessionRecordingMenuButton.tsx | Use generated SessionRecordingState enum. |
| src/panels/SessionRecordingPanel/RecordSession.tsx | Use generated SessionRecordingState enum. |
| src/panels/SessionRecordingPanel/Record/RecordingStopButton.tsx | Rename notification level enum usage. |
| src/panels/SessionRecordingPanel/PlaySession.tsx | Use generated SessionRecordingState enum. |
| src/panels/ScreenSpaceRenderablePanel/ImageTab.tsx | Rename notification level enum usage. |
| src/panels/Scene/SceneTree/hooks.ts | Switch filtered nodes type to PropertyOwnerRedux. |
| src/panels/NavigationPanel/NavigationPanel.tsx | Use generated EngineMode; migrate node type to PropertyOwnerRedux. |
| src/panels/NavigationPanel/MenuButton/NavigationPanelMenuButton.tsx | Use generated EngineMode. |
| src/panels/NavigationPanel/FocusView/FocusView.tsx | Use generated EngineMode; migrate owner types to PropertyOwnerRedux. |
| src/panels/NavigationPanel/FocusView/FocusEntry.tsx | Update entry prop type to PropertyOwnerRedux. |
| src/panels/NavigationPanel/AnchorAimView/AnchorAimView.tsx | Use generated EngineMode; migrate owner types to PropertyOwnerRedux. |
| src/panels/NavigationPanel/AnchorAimView/AnchorAimListEntry.tsx | Update node prop type to PropertyOwnerRedux. |
| src/panels/MissionsPanel/types.ts | Replace local mission types with generated MissionPhase/Milestone. |
| src/panels/MissionsPanel/Timeline/Timeline.tsx | Adapt timeline to MissionEntry/MissionPhase shapes. |
| src/panels/MissionsPanel/Timeline/Phases.tsx | Update phases rendering to generated MissionPhase. |
| src/panels/MissionsPanel/Timeline/PhaseRectangle.tsx | Update rectangle prop types to generated MissionPhase. |
| src/panels/MissionsPanel/Timeline/MilestonePolygon.tsx | Use generated Milestone type. |
| src/panels/MissionsPanel/MissionsPanel.tsx | Update mission selection labeling to new mission entry structure. |
| src/panels/MissionsPanel/MissionPhaseContent.tsx | Update content component to accept MissionEntry. |
| src/panels/MissionsPanel/MissionContent.tsx | Update mission content flow to use MissionEntry and generated phases. |
| src/panels/MissionsPanel/MissionCaptureButtons.tsx | Update capture time access to captureTimes on MissionEntry. |
| src/panels/Menu/TopMenuBar/Menus/ViewMenu.tsx | Rename log level UI to NotificationLevel. |
| src/panels/Menu/hooks.ts | Rename notification level enum usage in layout import flow. |
| src/panels/GettingStartedPanel/Tasks/SetPropertyTask.tsx | Switch property typing to openspace-api-js/types (PropertyValue). |
| src/panels/GeoLocationPanel/Search/GeoSearch.tsx | Rename notification level enum usage. |
| src/panels/GeoLocationPanel/hooks.ts | Rename notification level enum usage. |
| src/panels/FlightControlPanel/types.ts | Remove local flight controller command type file. |
| src/panels/FlightControlPanel/FlightController.tsx | Use generated flight controller input command shape. |
| src/panels/AssetsFolderPanel/AssetEntry/AssetsEntry.tsx | Use generated AssetLoadingData event payload shape. |
| src/panels/ActionsPanel/hooks.ts | Use generated Action type. |
| src/panels/ActionsPanel/ActionsPanel.tsx | Use generated Action type. |
| src/panels/ActionsPanel/ActionsButton.tsx | Use generated Action type. |
| src/hooks/sceneGraphNodes/hooks.ts | Return PropertyOwnerRedux[] and update imports. |
| src/hooks/propertyOwner.ts | Switch owner hooks to PropertyOwnerRedux types. |
| src/components/RecordingPlaybackOverlay/RecordingPlaybackOverlay.tsx | Use generated EngineMode enum. |
| src/components/Property/Types/VectorProperty/ViewOptions/VectorDefaultView.tsx | Use generated AdditionalDataVectorMatrix. |
| src/components/Property/Types/VectorProperty/ViewOptions/MinMaxRange.tsx | Use generated AdditionalDataVectorMatrix. |
| src/components/Property/Types/VectorProperty/ViewOptions/ColorView.tsx | Use generated AdditionalDataVectorMatrix. |
| src/components/Property/Types/VectorProperty/VectorProperty.tsx | Add explicit casts using new vector value helper types. |
| src/components/Property/Types/MatrixProperty.tsx | Add cast using new matrix value helper types. |
| src/components/Property/PropertyLabel.tsx | Use generated PropertyVisibility. |
| src/components/Property/PropertyDescription.tsx | Use generated PropertyVisibility. |
| src/components/NodeNavigationButton/NodeNavigationButton.tsx | Use generated EngineMode. |
| src/components/DynamicMap/hooks.ts | Update map hook input type to PropertyOwnerRedux. |
| src/components/About/About.tsx | Use generated SemanticVersion. |
| src/api/LuaApiProvider.tsx | Use generated OpenSpaceLibrary and call api.library(). |
| src/api/LuaApiContext.tsx | Update Lua API context type to OpenSpaceLibrary. |
Comments suppressed due to low confidence (1)
src/panels/MissionsPanel/MissionContent.tsx:75
- Calling
setPhaseToCurrent()during render (if (displayCurrentPhase) { ... }) performs state updates while rendering, which can cause extra renders or render loops and will trigger React warnings. Move this logic into auseEffectthat runs whendisplayCurrentPhase,now, or the phase list changes.
if (displayCurrentPhase) {
setPhaseToCurrent();
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function logNotificationMessage(logMessage: LogMessage, showNotification: boolean) { | ||
| const { category, level, message } = logMessage; | ||
|
|
||
| if (level === OpenSpaceLogLevel.NoLogging) { | ||
| if (!level || !category) { | ||
| return; | ||
| } |
| } | ||
|
|
||
| export interface PropertyOwner { | ||
| export interface PropertyOwnerRedux { |
There was a problem hiding this comment.
I find it a bit weird that we use a custom "Redux" version for the property owner type, but I see the point. Add a comment above on why this is needed? I.e. that this is a version that only includes the URIs for the proeprties and subowners rather than their full data.
It would be great if we didn't have to call this one "PropertyOwnerRedux" though, since it's the only one we actually use explicitly in the code, it seems. Is this possible?
There was a problem hiding this comment.
There is a real risk that you would import the wrong PropertyOwner since the API exports its own interface under the same name, hence why I decided to rename. But I'm down for alternative names or if we want to change the name of the exported type from the API. But maybe this also gives us the extra nudge to improve the PropertyOwner so that the data is exported in a different format
| logLevelStamping: true, | ||
| timeStamping: false, | ||
| categoryStamping: true, | ||
| dateStamping: false |
There was a problem hiding this comment.
You can now decide what additional data to get back in the callback. So if we want we could get the date and time as well but I didn't see a need for it currently. But could be useful when we decide to add a log panel
There was a problem hiding this comment.
Cool! Maybe consider renaming the keys to something more bool-like, such as useLogLevelStamping, useTimeStamping?
| <ColorView | ||
| value={value} | ||
| setPropertyValue={setPropertyValue} | ||
| setPropertyValue={(v) => setPropertyValue(v)} |
There was a problem hiding this comment.
| setPropertyValue={(v) => setPropertyValue(v)} | |
| setPropertyValue={setPropertyValue} |
| <MinMaxRangeView | ||
| value={value} | ||
| setPropertyValue={setPropertyValue} | ||
| setPropertyValue={(v) => setPropertyValue(v)} |
There was a problem hiding this comment.
| setPropertyValue={(v) => setPropertyValue(v)} | |
| setPropertyValue={setPropertyValue} |
| <VectorDefaultView | ||
| disabled={readOnly} | ||
| setPropertyValue={setPropertyValue} | ||
| setPropertyValue={(v) => setPropertyValue(v)} |
There was a problem hiding this comment.
| setPropertyValue={(v) => setPropertyValue(v)} | |
| setPropertyValue={setPropertyValue} |
| export interface Keybind { | ||
| action: string; | ||
| key: string; | ||
| export type KeybindRedux = Pick<Keybind, 'action' | 'key'> & { |
There was a problem hiding this comment.
Add a comment above this type and the PropertyOwnerRedux below
| logLevelStamping: true, | ||
| timeStamping: false, | ||
| categoryStamping: true, | ||
| dateStamping: false |
There was a problem hiding this comment.
Cool! Maybe consider renaming the keys to something more bool-like, such as useLogLevelStamping, useTimeStamping?
Description inc ™️