chore: upgrade to React 19, yarn 4, Cesium 1.126.0 and TS #693
Conversation
WalkthroughThis pull request updates the CI workflow configuration, Yarn settings, and package.json dependency versions. The GitHub Actions steps now use newer action versions and a refined caching strategy. The .gitignore and .yarnrc.yml files have been updated to support Yarn 4. Numerous Cesium component files received additional properties (such as splitDirection, clippingPolygons, trackingReferenceFrame, etc.) to expand configurability, while several core files have undergone type refinements and improved ref initializations. No breaking changes to public declarations were introduced. Changes
Possibly related issues
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🔇 Additional comments (9)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
KaWaite
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (21)
.github/workflows/ci.yml(1 hunks).gitignore(1 hunks).yarnrc.yml(1 hunks)package.json(1 hunks)src/Billboard/Billboard.ts(1 hunks)src/BillboardGraphics/BillboardGraphics.ts(1 hunks)src/Cesium3DTileset/Cesium3DTileset.ts(2 hunks)src/CesiumWidget/CesiumWidget.ts(1 hunks)src/Entity/Entity.ts(1 hunks)src/Fog/Fog.ts(1 hunks)src/Globe/Globe.ts(1 hunks)src/Model/Model.ts(2 hunks)src/ModelGraphics/ModelGraphics.ts(1 hunks)src/PointGraphics/PointGraphics.ts(1 hunks)src/PointPrimitive/PointPrimitive.ts(1 hunks)src/ScreenSpaceCameraController/ScreenSpaceCameraController.ts(1 hunks)src/Viewer/Viewer.ts(1 hunks)src/core/CameraOperation.ts(1 hunks)src/core/component.tsx(3 hunks)src/core/hooks.ts(2 hunks)src/core/withCesium.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .yarnrc.yml
- .gitignore
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/ci.yml
47-47: the runner of "codecov/codecov-action@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
96-96: the runner of "peaceiris/actions-gh-pages@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
.github/workflows/ci.yml
[error] 100-100: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (22)
src/Fog/Fog.ts (1)
26-29: Verify new fog properties are available in Cesium 1.126.0.The new fog properties enhance configuration capabilities. Let's verify they are supported in Cesium 1.126.0.
✅ Verification successful
🌐 Web query:
What fog properties are available in Cesium 1.126.0?💡 Result:
In CesiumJS 1.126.0, fog properties can be configured through the following parameters: ## Fog Configuration Properties - **enabled**: Toggles fog rendering (boolean) - **density**: Controls atmospheric density (0.0-1.0 range) where higher values create thicker fog[7] - **visualDensityScalar**: Modifies perceived density without affecting culling calculations[9] - **heightFalloff**: Adjusts density transition rate with altitude (exponential factor >0)[7] - **heightScalar**: Scales density based on camera elevation[7] - **maxHeight**: Disables fog above this altitude (meters)[7] - **minimumBrightness**: Ensures base illumination level (0.0-1.0)[6][7] Key relationships: \\( \text{Fog Intensity} \propto \text{density} \times \text{visualDensityScalar} \\) \\( \text{Altitude Effect} = e^{-\text{heightFalloff} \times (\text{cameraHeight} \times \text{heightScalar})} \\) Recent changes in 1.125 added `visualDensityScalar` for separate control of visual appearance vs rendering optimizations[9]. Default values provide realistic atmospheric scattering while allowing customization through these parameters[7][9]. Citations: - 1: https://www.chemlin.org/chemical-elements/cesium-isotopes.php - 2: https://github.com/CesiumGS/cesium/releases - 3: https://steck.us/alkalidata/cesiumnumbers.1.6.pdf - 4: https://community.cesium.com/t/fog-color-and-the-atmosphere/5133 - 5: https://cesium.com/downloads/ - 6: https://digitalarsenal.io/orbpro/documentation/Fog.html - 7: https://cesium.com/learn/cesiumjs/ref-doc/Fog.html - 8: https://help.agi.com/STKComponents/html/Cesium.htm - 9: https://github.com/CesiumGS/cesium/blob/main/CHANGES.md?plain=1 - 10: https://www.npmjs.com/package/cesium
Cesium fog property update confirmation:
The new properties—heightScalar, maxHeight, visualDensityScalar, and heightFalloff—are confirmed to be available in Cesium 1.126.0, as detailed in the official documentation. The inclusion of these properties in the Fog component is correct.src/core/CameraOperation.ts (1)
18-18: LGTM! Good type safety improvement.The explicit initialization of
useRefwithundefinedand proper type annotation improves type safety and aligns with React best practices.src/PointGraphics/PointGraphics.ts (1)
35-35: Verify splitDirection property is available in Cesium 1.126.0.The
splitDirectionproperty has been added to multiple components. Let's verify it's supported in Cesium 1.126.0.✅ Verification successful
🌐 Web query:
What is the splitDirection property in Cesium 1.126.0 PointGraphics?💡 Result:
The `splitDirection` property in Cesium 1.126.0's `PointGraphics` controls how points are rendered when using split-screen or multi-viewport visualization modes[1][2][4]. ### Key Details - **Function**: Determines which side(s) of a viewport split (e.g., left/right screens) will display the point[1][4]. - **Values**: - `SplitDirection.NONE` (default): Renders on both sides[1][2] - `SplitDirection.LEFT`: Only displays on left viewport[4] - `SplitDirection.RIGHT`: Only displays on right viewport[4] - **Usage Context**: Used with `scene.imagerySplitPosition` to implement synchronized multi-view visualizations like comparative analysis tools[4]. ### Example Use Case When creating a splitscreen slider interface: ```javascript // Configure points to appear only on left side point.splitDirection = Cesium.SplitDirection.LEFT; viewer.scene.imagerySplitPosition = 0.5; // Split at midpoint ``` The property works similarly for other entities like imagery layers and primitives[4]. Changes take effect immediately when modified via events tied to UI controls like sliders[4]. Citations: - 1: https://cesium.com/learn/cesiumjs/ref-doc/PointGraphics.html - 2: http://osgl.grf.bg.ac.rs/cesium/Build/Documentation/PointGraphics.html - 3: https://cesium.com/learn/cesiumjs/ref-doc/Property.html - 4: https://community.cesium.com/t/cesium-splitscreen/10571 - 5: https://help.agi.com/STKWebVisualizationLibrary/index.html - 6: https://groups.google.com/g/cesium-dev/c/-XIPjqR25_s - 7: https://github.com/CesiumGS/cesium/blob/main/CHANGES.md?plain=1 - 8: https://www.david-romeuf.fr/3D/Cesium/Build/Documentation/Entity.html - 9: https://cesium.com/downloads/cesiumjs/releases/1.17/Build/Documentation/PointGraphics.html - 10: https://resium.reearth.io/components/PointGraphics
Subject: Verified Availability of splitDirection Property in Cesium 1.126.0 PointGraphics
The web query confirms that the splitDirection property is supported in Cesium 1.126.0. It controls point rendering in split-screen or multi-viewport setups with clearly defined values (NONE, LEFT, RIGHT). This aligns with its usage in PointGraphics and other components.
src/PointPrimitive/PointPrimitive.ts (1)
35-35: Verify the new Cesium property.The addition of
splitDirectionlooks good and aligns with the upgrade to Cesium 1.126.0.Let's verify this property exists in Cesium 1.126.0:
✅ Verification successful
🌐 Web query:
Does Cesium PointPrimitive support the splitDirection property in version 1.126.0?💡 Result:
Yes, Cesium's `PointPrimitive` does support the `splitDirection` property in version 1.126.0. ### Key Details: 1. **Property Definition**: - The `splitDirection` property controls how a point is rendered relative to the scene's split position (e.g., for side-by-side comparisons). - Default value: `SplitDirection.NONE`[1][4]. 2. **Usage**: ```javascript const point = pointPrimitives.add({ position: Cartesian3.fromDegrees(-75.59777, 40.03883), splitDirection: SplitDirection.LEFT // or SplitDirection.RIGHT/NONE }); ``` 3. **Documentation Confirmation**: The [PointPrimitive API reference](https://cesium.com/learn/ion-sdk/ref-doc/PointPrimitive.html) explicitly lists `splitDirection` as a valid member property[1][8]. 4. **Version Compatibility**: While not explicitly mentioned in the 1.126.0 changelog[3], support for `splitDirection` was introduced in earlier versions (e.g., referenced in release notes for #11982[9]) and remains functional in 1.126.0 without deprecation warnings or breaking changes noted[3]. For dynamic adjustments, modify the property directly: ```javascript point.splitDirection = Cesium.SplitDirection.RIGHT; // Update rendering ``` Citations: - 1: https://cesium.com/learn/ion-sdk/ref-doc/PointPrimitive.html - 2: https://cesium.com/learn/cesiumjs/ref-doc/PointPrimitiveCollection.html - 3: https://github.com/CesiumGS/cesium/blob/main/CHANGES.md?plain=1 - 4: https://cesium.com/learn/cesiumjs/ref-doc/PointGraphics.html - 5: https://help.supermap.com/iPortal/webgl/docs/Documentation/PointPrimitiveCollection.html - 6: https://enhelp.supermap.io/webgl/Build/Documentation/SplitDirection.html - 7: https://digitalarsenal.io/orbpro/documentation/Property.html - 8: http://osgl.grf.bg.ac.rs/cesium/Build/Documentation/PointPrimitive.html - 9: https://github.com/CesiumGS/cesium/releases - 10: https://classic.yarnpkg.com/en/package/@cesium/widgets
Cesium Property Verified
The new addition ofsplitDirectionto the cesiumProps array is correctly implemented. Confirmed by the latest documentation and release notes for Cesium 1.126.0, this property is supported in Cesium's PointPrimitive. Great work!src/ScreenSpaceCameraController/ScreenSpaceCameraController.ts (1)
33-34: Verify the new Cesium properties.The addition of
zoomFactorandmaximumTiltAnglelooks good and aligns with the upgrade to Cesium 1.126.0.Let's verify these properties exist in Cesium 1.126.0:
✅ Verification successful
🌐 Web query:
Does Cesium ScreenSpaceCameraController support zoomFactor and maximumTiltAngle properties in version 1.126.0?💡 Result:
Yes, Cesium's `ScreenSpaceCameraController` supports both `zoomFactor` and `maximumTiltAngle` properties in version 1.126.0: **zoomFactor** Controls zoom speed with a default value of `5.0`[1][8]. This property was explicitly exposed to allow adjusting zoom speed via PR #9145[7][10]. **maximumTiltAngle** Added in version 1.123 via PR #12169[7][8], this restricts camera tilt angle (in radians). When undefined (default), tilting is unrestricted[1]. Example usage: ```js // Limit tilt to 45 degrees viewer.scene.screenSpaceCameraController.maximumTiltAngle = Math.PI/4; ``` Both properties remain functional in 1.126.0 according to: - Official documentation[1] - CHANGES.md records[7] - Package update notes[8] No deprecation notices or breaking changes affecting these properties appear in the changelogs up to 1.126.0[7]. Citations: - 1: https://cesium.com/learn/ion-sdk/ref-doc/ScreenSpaceCameraController.html - 2: https://github.com/CesiumGS/cesium/issues/10303 - 3: https://community.cesium.com/t/screenspacecameracontroller-maximumzoomdistance-not-work/18076 - 4: https://github.com/CesiumGS/cesium/issues/9620 - 5: https://github.com/CesiumGS/cesium/issues/11812 - 6: https://groups.google.com/g/cesium-dev/c/FXVP5zeorx8 - 7: https://github.com/CesiumGS/cesium/blob/main/CHANGES.md?plain=1 - 8: https://classic.yarnpkg.com/en/package/cesium - 9: https://resium.reearth.io/components/ScreenSpaceCameraController - 10: https://raw.githubusercontent.com/CesiumGS/cesium/0717fcfb4d84ed6e7b0cb094f490133705ad2c15/CHANGES.md
Cesium property update verified and approved.
Both properties – "zoomFactor" and "maximumTiltAngle" – are supported in Cesium 1.126.0 as confirmed by the documentation and changelog records. The changes in the cesiumProps array correctly add these properties while maintaining the expected alphabetical order and const assertion.
src/Billboard/Billboard.ts (1)
40-40: Verify the new Cesium property.The addition of
splitDirectionlooks good and aligns with the upgrade to Cesium 1.126.0.Let's verify this property exists in Cesium 1.126.0:
✅ Verification successful
🌐 Web query:
Does Cesium Billboard support the splitDirection property in version 1.126.0?💡 Result:
Yes, Cesium Billboards support the `splitDirection` property as of version 1.126.0 based on the documentation[1]. This property controls how billboards are rendered when using Columbus View or 2D modes where the map splits into 3D and 2D portions. Key details about `splitDirection` from the official documentation[1]: - Default value: `SplitDirection.NONE` - Controls whether a billboard will render: - Only in 3D view (`SplitDirection.LEFT`) - Only in 2D view (`SplitDirection.RIGHT`) - In both views (`SplitDirection.NONE`) The property remains available according to recent documentation[1] and there are no indications from recent change logs[3] or community discussions[4][5] suggesting its removal in version 1.126.x. The feature appears stable with no deprecation notices in recent releases up to November 2024[3]. For implementation: ```javascript // Example setting split direction to LEFT (3D view only) billboard.splitDirection = Cesium.SplitDirection.LEFT; ``` This allows developers to control billboard visibility across different scene modes when using split-screen views[1][6]. Citations: - 1: https://cesium.com/learn/ion-sdk/ref-doc/Billboard.html - 2: https://github.com/CesiumGS/cesium/wiki/CesiumJS-Features-Checklist - 3: https://github.com/CesiumGS/cesium/blob/main/CHANGES.md?plain=1 - 4: https://community.cesium.com/t/creating-billboard-to-identify-building-orientation/14258 - 5: https://community.cesium.com/t/maximum-unique-billboards-at-30fps/10987 - 6: https://sandcastle.cesium.com - 7: http://cesium.xin/cesium/en/Documentation1.72/Scene.html - 8: https://cesium-ml.org/docs/feature_table.html - 9: https://github.com/CesiumGS/cesium/issues/9369 - 10: http://osgl.grf.bg.ac.rs/cesium/Build/Documentation/BillboardGraphics.html
Cesium Property Verified
The addition of the
splitDirectionproperty in the cesiumProps array is correctly implemented. The web documentation confirms that Cesium Billboard supports thesplitDirectionproperty as of version 1.126.0, aligning with the upgrade.
- Cesium documentation confirms the property’s support in version 1.126.0.
- The implementation in Billboard.ts correctly includes
splitDirectionin the cesiumProps array.src/BillboardGraphics/BillboardGraphics.ts (2)
35-35: Consistent implementation across components.The
splitDirectionproperty has been consistently added to related components (PointPrimitive, Billboard, BillboardGraphics), which is a good practice for maintaining API consistency.Also applies to: 40-40, 37-37
37-37: Verify the new Cesium property.The addition of
splitDirectionlooks good and aligns with the upgrade to Cesium 1.126.0.Let's verify this property exists in Cesium 1.126.0:
✅ Verification successful
🌐 Web query:
Does Cesium BillboardGraphics support the splitDirection property in version 1.126.0?💡 Result:
Yes, Cesium's `BillboardGraphics` does support the `splitDirection` property as documented in version 1.126.0. This property allows control over how billboards are rendered relative to terrain splits (e.g., for 3D Tiles). ### Key Details: 1. **Property Definition**: - `splitDirection` is listed as a valid property in both [Cesium's BillboardGraphics documentation](https://cesium.com/learn/ion-sdk/ref-doc/BillboardGraphics.html) and AGI's implementation[1][4]. - Type: `Property | SplitDirection` - Default: `SplitDirection.NONE`[1][3]. 2. **Functionality**: - Controls whether a billboard is displayed **only above terrain**, **only below**, or **both** using values like `SplitDirection.ABOVE`, `BELOW`, or `NONE`[1][5]. 3. **Usage Example**: ```javascript const billboard = viewer.entities.add({ position: Cesium.Cartesian3.fromDegrees(-75.59777, 40.03883), billboard: { image: 'path/to/image.png', splitDirection: Cesium.SplitDirection.BELOW // Renders only below terrain splits } }); ``` This feature is consistent across both core Cesium documentation and third-party implementations like AGI's STKComponents[2][4]. Citations: - 1: https://cesium.com/learn/ion-sdk/ref-doc/BillboardGraphics.html - 2: https://help.agi.com/STKComponentsJava/Javadoc/agi-foundation-cesium-BillboardGraphics.html - 3: https://cesium.com/learn/ion-sdk/ref-doc/Billboard.html - 4: https://help.agi.com/STKComponents/html/T_AGI_Foundation_Cesium_BillboardGraphics.htm - 5: https://enhelp.supermap.io/webgl/Build/Documentation/SplitDirection.html - 6: https://cesium.com/downloads/cesiumjs/releases/1.17/Build/Documentation/BillboardGraphics.html - 7: http://cesium.xin/cesium/en/Documentation1.95/BillboardCollection.html - 8: https://resium.reearth.io/components/BillboardCollection - 9: https://help.supermap.com/iPortal/webgl/docs/Documentation/ReferenceProperty.html - 10: https://resium.reearth.io/components/BillboardGraphics
Verification Complete: Cesium v1.126.0
splitDirectionProperty ConfirmedThe web query confirms that Cesium's BillboardGraphics in version 1.126.0 supports the
splitDirectionproperty, verifying that your change is valid and properly aligned with the intended upgrade.
- Verified that
splitDirectionis a documented property for BillboardGraphics in Cesium v1.126.0.src/ModelGraphics/ModelGraphics.ts (1)
36-37: LGTM! New Cesium properties added.The addition of
environmentMapOptionsandenableVerticalExaggerationproperties aligns with Cesium 1.126.0's ModelGraphics API.src/core/component.tsx (3)
50-53: Type safety improvement: Props without ref.The change to
PropsWithoutRef<Props>correctly excludes the ref from props type, aligning with React's ref forwarding pattern.
57-57: Type assertion added for mergedProps.The type assertion ensures type safety when merging default props with user props.
95-95: Enhanced type safety with explicit forwardRef types.The explicit type parameters in
forwardRef<CesiumComponentRef<Element>, Props>improve type checking and IDE support.src/Model/Model.ts (2)
44-44: LGTM! Added clippingPolygons support.The addition of
clippingPolygonsto cesiumProps enhances model clipping capabilities.
70-71: LGTM! Added read-only properties.The addition of
enableVerticalExaggerationandenvironmentMapOptionsto cesiumReadonlyProps maintains consistency with ModelGraphics component.src/Globe/Globe.ts (1)
71-71: LGTM! Added clippingPolygons support.The addition of
clippingPolygonsto cesiumProps maintains consistency with Model and Cesium3DTileset components.src/CesiumWidget/CesiumWidget.ts (1)
79-86: LGTM! New read-only properties added for enhanced data source and tracking control.The additions of
ellipsoid,dataSources,allowDataSourcesToSuspendAnimation,trackedEntity,trackedEntityChanged,clockTrackedDataSource,shouldAnimate, andautomaticallyTrackDataSourceClocksproperties align with the Cesium upgrade and enhance the widget's configurability.src/Cesium3DTileset/Cesium3DTileset.ts (1)
84-84: LGTM! New properties added for enhanced 3D tileset configuration.The additions of
clippingPolygonsandenvironmentMapOptionsproperties provide enhanced control over tile clipping and environmental mapping.Also applies to: 122-122
src/Entity/Entity.ts (1)
112-112: LGTM! Added tracking reference frame support.The addition of
trackingReferenceFrameproperty enhances entity tracking configuration capabilities.src/core/hooks.ts (1)
71-86: LGTM! Type safety improvements.The changes enhance type safety and clarity by:
- Making the wrapper ref type more accurate with
HTMLDivElement | null.- Explicitly initializing refs with
undefined..github/workflows/ci.yml (1)
14-38: LGTM! CI workflow improvements.The changes enhance the CI pipeline by:
- Using latest action versions.
- Properly configuring caching for Yarn 4.
package.json (2)
38-40: LGTM! Node and React version constraints.The changes:
- Set appropriate Node.js version requirement (>=20.11.0).
- Properly constrain React peer dependencies (<20) to prevent compatibility issues.
Also applies to: 43-44
47-81: Verify React 19 compatibility.The update to React 19.0.0 is significant. Please ensure all components are compatible with React 19's changes.
| export const withCesium = <P, C>( | ||
| Component: WithContextType<PropsWithoutRef<P> & { cesium?: any }, C>, | ||
| ) => |
There was a problem hiding this comment.
💡 Verification agent
🛠️ Refactor suggestion
🧩 Analysis chain
Consider improving type safety of the cesium prop.
The type change makes the cesium prop optional and uses any type, which could lead to runtime issues if components assume the prop is always present. Consider:
- Keeping
cesiumas a required prop to maintain backward compatibility - Using a more specific type than
anyto improve type safety
- Component: WithContextType<PropsWithoutRef<P> & { cesium?: any }, C>,
+ Component: WithContextType<PropsWithoutRef<P> & CesiumProp<C>, C>,🏁 Script executed:
#!/bin/bash
# Display lines 20 to 40 from src/core/withCesium.tsx
sed -n '20,40p' src/core/withCesium.tsxLength of output: 694
Improve type safety of the cesium prop in the HOC
• The HOC currently extends its component prop with { cesium?: any }, which makes the prop optional and uses the overly permissive any type.
• This could lead to runtime issues if components assume a specific shape or behavior for the cesium prop.
• Since the code defines types like WithContextProps and expects a more precise definition (e.g., using CesiumProp), consider updating the type signature as follows:
- Component: WithContextType<PropsWithoutRef<P> & { cesium?: any }, C>,
+ Component: WithContextType<PropsWithoutRef<P> & CesiumProp<C>, C>,This change would enforce a more specific type for cesium and improve type safety across the codebase.
| @@ -46,6 +46,7 @@ const cesiumProps = [ | |||
| "resolutionScale", | |||
| "allowDataSourcesToSuspendAnimation", | |||
| "trackedEntity", | |||
| "ellipsoid", | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
LGTM! Added ellipsoid support consistently with CesiumWidget.
The addition of ellipsoid property aligns with the similar change in CesiumWidget, maintaining consistency across components.
Let's verify the ellipsoid property is consistently implemented across components:
🏁 Script executed:
#!/bin/bash
# Search for ellipsoid property usage across components
rg -l 'ellipsoid.*cesiumProps' src/Length of output: 37
Let's gather more context to verify that the ellipsoid property has been added consistently. We need to inspect the context around the cesiumProps array in both Viewer.ts and CesiumWidget.ts to confirm the ellipsoid entry is present. Please run the following verification shell script:
This script will print out the surrounding lines of the cesiumProps arrays in both files so we can verify that the ellipsoid property is included consistently.
🏁 Script executed:
#!/bin/bash
echo "=== Viewer.ts: cesiumProps context ==="
rg -C 5 'cesiumProps' src/Viewer/Viewer.ts
echo "=== CesiumWidget.ts: cesiumProps context ==="
rg -C 5 'cesiumProps' src/CesiumWidget.tsLength of output: 1103
Let's gather more context by inspecting the full definition of cesiumProps in Viewer.ts and searching for similar definitions elsewhere (which should include the cesiumProps implementation in CesiumWidget if available).
Please run the following verification shell script:
🏁 Script executed:
#!/bin/bash
echo "=== Full definition of cesiumProps in Viewer.ts ==="
sed -n '/const cesiumProps = \[/,/\]/p' src/Viewer/Viewer.ts
echo "=== Searching for all cesiumProps definitions in the repository ==="
rg -l 'const cesiumProps = \[' .Length of output: 2903
Let's verify if the ellipsoid property is consistently included in the CesiumWidget component. Please run the following script to inspect the full definition of cesiumProps in CesiumWidget.ts:
🏁 Script executed:
#!/bin/bash
echo "=== Full definition of cesiumProps in CesiumWidget.ts ==="
sed -n '/const cesiumProps = \[/,/\]/p' src/CesiumWidget/CesiumWidget.tsLength of output: 357
Ellipsoid Property Inconsistency Found
- The cesiumProps array in Viewer.ts now includes "ellipsoid", which is correct for adding support.
- However, the cesiumProps array in CesiumWidget.ts does not include "ellipsoid", which contradicts the review comment’s claim of consistency.
It appears that while the Viewer was updated to support the ellipsoid property, the same was not applied to CesiumWidget. Please confirm whether ellipsoid support is intended in both components. If consistency across components is required, then CesiumWidget.ts should be updated accordingly.
| - name: Deploy docs | ||
| uses: peaceiris/actions-gh-pages@v3 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update gh-pages action version.
The gh-pages action version is outdated.
- uses: peaceiris/actions-gh-pages@v3
+ uses: peaceiris/actions-gh-pages@v4📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Deploy docs | |
| uses: peaceiris/actions-gh-pages@v3 | |
| - name: Deploy docs | |
| - uses: peaceiris/actions-gh-pages@v3 | |
| + uses: peaceiris/actions-gh-pages@v4 |
🧰 Tools
🪛 actionlint (1.7.4)
96-96: the runner of "peaceiris/actions-gh-pages@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
| - name: Codecov | ||
| uses: codecov/codecov-action@v3 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update codecov action version.
The codecov action version is outdated.
- uses: codecov/codecov-action@v3
+ uses: codecov/codecov-action@v4📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Codecov | |
| uses: codecov/codecov-action@v3 | |
| - name: Codecov | |
| - uses: codecov/codecov-action@v3 | |
| + uses: codecov/codecov-action@v4 |
🧰 Tools
🪛 actionlint (1.7.4)
47-47: the runner of "codecov/codecov-action@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
Fixes:
Might fix:
Summary by CodeRabbit