Repository navigation
bump to 2.2.6 and variable support - #49
Conversation
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughThis PR adds declarative scene variables and preset support to unicornstudio-react. It expands public types, updates ChangesVariables and Presets Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Component
participant useUnicornScene
participant Scene
Component->>useUnicornScene: pass variables, preset, onVariableChange
useUnicornScene->>Scene: addScene(initialVariables, initialPreset)
Scene-->>useUnicornScene: onVariableChange unsubscribe
Component->>useUnicornScene: update variables or preset
useUnicornScene->>Scene: setVariables() / setPreset()
Scene-->>useUnicornScene: variable change event
useUnicornScene-->>Component: onVariableChange callback
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/__tests__/useUnicornScene.test.ts (1)
470-493: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider asserting no re-initialization in the graceful-handling test.
The test verifies that rerendering with new
variables/preset/onVariableChangedoesn't throw when the scene lacks the variables API, but it doesn't verify thataddSceneMockwas called only once. Adding this assertion would confirm the hook correctly avoids scene re-creation in this edge case, matching the pattern used in the other sync tests.🧪 Suggested addition
await act(async () => {}); + expect(addSceneMock).toHaveBeenCalledTimes(1); + expect(() => { rerender({ ...defaultProps(elementRef), variables: { intensity: 0.9 }, preset: "Light Theme", onVariableChange: vi.fn(), }); }).not.toThrow(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/useUnicornScene.test.ts` around lines 470 - 493, In the “handles scenes without the variables API gracefully” test, also assert that the scene is not re-initialized on rerender. After the existing no-throw check in useUnicornScene, verify addSceneMock was called only once so the hook’s graceful path still avoids recreating the scene when variables/preset/onVariableChange change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 334-336: The README table entry for the `variables` prop uses a
placeholder type and should be updated to the exported public type. Replace the
`Record<string, value>` description with `UnicornVariables` in the documentation
section that lists `variables`, `preset`, and `onVariableChange`, so it matches
the actual API surface.
In `@src/shared/hooks.ts`:
- Around line 405-406: The sync path for `variables` and `preset` can fire
before `internalSceneRef.current` is ready during `addScene()`, so the latest
values are never applied to the newly created scene. Update the logic around the
`variablesRef`/`presetRef` sync effects in `hooks.ts` so they re-run or replay
once `internalSceneRef.current` becomes available after async scene creation,
ensuring the final scene gets the latest `variables` and `preset` instead of
stale initial values.
---
Nitpick comments:
In `@src/__tests__/useUnicornScene.test.ts`:
- Around line 470-493: In the “handles scenes without the variables API
gracefully” test, also assert that the scene is not re-initialized on rerender.
After the existing no-throw check in useUnicornScene, verify addSceneMock was
called only once so the hook’s graceful path still avoids recreating the scene
when variables/preset/onVariableChange change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 571d301f-61ef-4ed1-accb-bc083fa10168
📒 Files selected for processing (12)
CHANGELOG.mdREADME.mdpackage.jsonsrc/__tests__/useUnicornScene.test.tssrc/index.tsxsrc/next/index.tsxsrc/react/index.tsxsrc/shared/constants.tssrc/shared/hooks.tssrc/shared/sdk-bundle.tssrc/shared/types.tsvendor/unicornstudio/unicornStudio.umd.js
buildSceneConfig snapshots variablesRef/presetRef before awaiting addScene(). If either prop changed while creation was in flight, the sync effects ran against a still-null scene ref and no-op'd, leaving the scene with stale initial values and no replay. Capture what the scene was created with, then apply any drift once addScene() resolves. Also sync package-lock version to 2.2.6 and correct the README variables type to the exported UnicornVariables.
Conflicts were version-only: - package.json / package-lock.json: keep 2.2.6 - src/shared/sdk-bundle.ts: generated file, regenerated from vendor/
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/__tests__/useUnicornScene.test.ts (1)
470-504: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider asserting exact call counts for the replay path.
The test correctly verifies that changed
variables/presetare replayed viasetVariables/setPresetafter pending scene creation resolves. However,toHaveBeenCalledWithonly checks that the function was called with the given args at least once — it would still pass if the sync effects also fired and double-called these methods. AddingtoHaveBeenCalledTimes(1)would lock down the replay path and catch potential double-call regressions.🧪 Suggested additions
expect(setVariables).toHaveBeenCalledWith({ intensity: 0.9 }); expect(setPreset).toHaveBeenCalledWith("Light Theme"); + expect(setVariables).toHaveBeenCalledTimes(1); + expect(setPreset).toHaveBeenCalledTimes(1); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/useUnicornScene.test.ts` around lines 470 - 504, The replay-path test in useUnicornScene is only checking arguments, so it could still pass if setVariables or setPreset are called multiple times by both the replay logic and the normal sync effects. Update the assertion block in this test to also verify exact call counts for setVariables and setPreset, keeping the focus on the pending addScene resolution path and guarding against double-call regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/__tests__/useUnicornScene.test.ts`:
- Around line 470-504: The replay-path test in useUnicornScene is only checking
arguments, so it could still pass if setVariables or setPreset are called
multiple times by both the replay logic and the normal sync effects. Update the
assertion block in this test to also verify exact call counts for setVariables
and setPreset, keeping the focus on the pending addScene resolution path and
guarding against double-call regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ee83063c-1c18-482d-ac6e-50a4a1c4cb50
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
README.mdsrc/__tests__/useUnicornScene.test.tssrc/shared/constants.tssrc/shared/hooks.ts
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/shared/constants.ts
- src/shared/hooks.ts
The replay branches pushed initializeScene's cognitive complexity to 21, over the 15 allowed. Move the snapshot and the replay into helpers. No behavior change.
|
2 participants
Gave this one to Fable:
What the integration looks like
Three new declarative props, available on both the React and Next.js components:
The design deliberately mirrors the existing paused prop pattern rather than inventing anything new:
For everything beyond that (GSAP-driven variables, building control panels from the manifest, direct layer control), the existing sceneRef prop is now the typed escape hatch — UnicornStudioScene gained the full 2.2 runtime surface: setVariable, getVariableManifest, getPresets, setProp, setTexture, getLayers, getLayer, getMouse, etc. All new methods are optional on the interface so a custom sdkUrl pointing at an older SDK degrades gracefully instead of throwing.
Summary by CodeRabbit
New Features
variablesandpresetsupport, including initial application, live syncing, andonVariableChangecallbacks.Documentation
Tests
Chores