-
-
Notifications
You must be signed in to change notification settings - Fork 2
docs: changelog updated version 0.0.2 #11
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
Conversation
WalkthroughThis update introduces explicit LF line endings in Prettier configuration, adds a changelog entry for version 0.0.2, updates the example to use an external package and provide quiz cues, refactors the player initialization and cue/translation loading logic in the main component, and migrates the tsup build config to a functional form with dynamic output extension handling. Changes
Sequence Diagram(s)sequenceDiagram
participant App as React App
participant IV as InteractiveVideo Component
participant Player as IVLabsPlayer
App->>IV: Render with props (cues, translations)
IV->>Player: Initialize player if container exists
IV->>Player: Register event listeners
IV->>Player: Load cues (on cues prop change)
IV->>Player: Load translations (on translations prop change)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 5
🧹 Nitpick comments (2)
examples/index.tsx (1)
16-26: Validate cue shape against coreCuePointtype.The cue uses a
payloadwith{ type: 'quiz', ... }and omits a top-leveltype. IfCuePointexpects a top-leveltype, this may be rejected at runtime.If needed, adjust to match the expected shape (e.g., add
typeat the top level) per@interactive-video-labs/core’s type:{ id: 'cue1', time: 2, // type: 'quiz', // if required at top-level payload: { type: 'quiz', question: '...', answers: [...] } }Tip: In real browsers, autoplay often requires
mutedto be true to avoid being blocked; consider adding it in the example for a smoother OOTB run.CHANGELOG.md (1)
15-16: Fix typos and polish phrasing.
- “confugured” → “configured”
- Consider capitalizing “LF” for consistency.
- - Tsup config file confugured + - Tsup config file configured
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.prettierrc(1 hunks)CHANGELOG.md(1 hunks)examples/index.tsx(2 hunks)src/index.tsx(2 hunks)tsup.config.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/index.tsx (1)
test/InteractiveVideo.test.tsx (5)
videoSrc(34-85)IVLabsPlayer(13-20)loadCues(14-19)beforeEach(22-93)render(87-92)
🔇 Additional comments (1)
examples/index.tsx (1)
3-3: Example import from published package looks good.This aligns the example with the external package entry point and validates the public API surface.
| "singleQuote": true, | ||
| "endOfLine": "lf" |
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.
💡 Verification agent
🧩 Analysis chain
Enforce LF consistently across OS (add .gitattributes).
Prettier will rewrite files to LF on save, but Git checkouts on Windows can still introduce CRLF. Add a repo-level .gitattributes to avoid churn.
Suggested .gitattributes:
* text=auto eol=lf
# binary files
*.png -text
*.jpg -text
*.jpeg -text
*.gif -text
*.pdf -text
Verification script to confirm presence/config:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if fd -t f ".gitattributes" >/dev/null; then
echo ".gitattributes found"; echo
fd -t f ".gitattributes" -x sh -c 'echo "== {} =="; cat {}'
else
echo "Missing .gitattributes at repo root"
exit 1
fiLength of output: 163
Enforce LF line endings via .gitattributes
The .gitattributes file exists at the repo root but is currently empty. To avoid CRLF churn on Windows checkouts, please add the following content:
Files needing update:
.gitattributes(repo root)
Suggested diff:
--- .gitattributes
+++ .gitattributes
+* text=auto eol=lf
+
+# binary files
+*.png -text
+*.jpg -text
+*.jpeg -text
+*.gif -text
+*.pdf -text🤖 Prompt for AI Agents
In the .prettierrc file at lines 5 to 6, the review suggests enforcing LF line
endings via the .gitattributes file. To fix this, create or update the
.gitattributes file at the repository root by adding configuration that enforces
LF line endings for all relevant files, preventing CRLF line ending issues on
Windows checkouts.
| if (!containerRef.current) return; | ||
|
|
||
| try { | ||
| setTimeout(() => { | ||
| if (containerRef.current) { | ||
| const player = new IVLabsPlayer(uniqueIdRef.current, playerConfig); | ||
| playerRef.current = player; | ||
|
|
||
| if (onAnalyticsEvent) { | ||
| player.on('PLAYER_LOADED', (payload?: AnalyticsPayload) => | ||
| onAnalyticsEvent('PLAYER_LOADED', payload) | ||
| ); | ||
| player.on('VIDEO_STARTED', (payload?: AnalyticsPayload) => | ||
| onAnalyticsEvent('VIDEO_STARTED', payload) | ||
| ); | ||
| player.on('VIDEO_PAUSED', (payload?: AnalyticsPayload) => | ||
| onAnalyticsEvent('VIDEO_PAUSED', payload) | ||
| ); | ||
| player.on('VIDEO_ENDED', (payload?: AnalyticsPayload) => | ||
| onAnalyticsEvent('VIDEO_ENDED', payload) | ||
| ); | ||
| player.on('CUE_TRIGGERED', (payload?: AnalyticsPayload) => | ||
| onAnalyticsEvent('CUE_TRIGGERED', payload) | ||
| ); | ||
| player.on('INTERACTION_COMPLETED', (payload?: AnalyticsPayload) => | ||
| onAnalyticsEvent('INTERACTION_COMPLETED', payload) | ||
| ); | ||
| player.on('ERROR', (payload?: AnalyticsPayload) => | ||
| onAnalyticsEvent('ERROR', payload) | ||
| ); | ||
| } | ||
| const playerConfig: PlayerConfig = { | ||
| videoUrl, | ||
| ...restOptions, | ||
| }; | ||
|
|
||
| if (cues) { | ||
| player.loadCues(cues); | ||
| } | ||
| try { | ||
| const player = new IVLabsPlayer(uniqueIdRef.current, playerConfig); | ||
| playerRef.current = player; | ||
|
|
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.
🛠️ Refactor suggestion
Avoid re-instantiating the player on every render; stabilize dependencies and callback.
Including restOptions and onAnalyticsEvent in the init effect deps makes the effect run frequently (new object/callback identity), tearing down and recreating the player unnecessarily. This can cause flicker, state loss, and missed cue/translation loads.
Apply these changes:
- Initialize/destroy the player only when
videoUrlchanges. - Use a ref to hold the latest
onAnalyticsEventto avoid re-binding/recreating.
Diff within this block:
- useEffect(() => {
- if (!containerRef.current) return;
-
- const playerConfig: PlayerConfig = {
- videoUrl,
- ...restOptions,
- };
-
- try {
- const player = new IVLabsPlayer(uniqueIdRef.current, playerConfig);
- playerRef.current = player;
-
- if (onAnalyticsEvent) {
- const events: AnalyticsEvent[] = [
- 'PLAYER_LOADED',
- 'VIDEO_STARTED',
- 'VIDEO_PAUSED',
- 'VIDEO_ENDED',
- 'CUE_TRIGGERED',
- 'INTERACTION_COMPLETED',
- 'ERROR',
- ];
- events.forEach((event) => {
- player.on(event, (payload?: AnalyticsPayload) =>
- onAnalyticsEvent(event, payload)
- );
- });
- }
- } catch (error) {
- console.error('Error initializing IVLabsPlayer:', error);
- }
-
- return () => {
- if (playerRef.current) {
- playerRef.current.destroy();
- playerRef.current = null;
- }
- };
- }, [videoUrl, onAnalyticsEvent, restOptions]);
+ useEffect(() => {
+ if (!containerRef.current) return;
+ try {
+ // Recreate only when videoUrl changes
+ if (playerRef.current) {
+ playerRef.current.destroy();
+ playerRef.current = null;
+ }
+ const playerConfig: PlayerConfig = { videoUrl, ...restOptions };
+ const player = new IVLabsPlayer(uniqueIdRef.current, playerConfig);
+ playerRef.current = player;
+
+ const events: AnalyticsEvent[] = [
+ 'PLAYER_LOADED',
+ 'VIDEO_STARTED',
+ 'VIDEO_PAUSED',
+ 'VIDEO_ENDED',
+ 'CUE_TRIGGERED',
+ 'INTERACTION_COMPLETED',
+ 'ERROR',
+ ];
+ events.forEach((event) => {
+ player.on(event, (payload?: AnalyticsPayload) => {
+ const cb = onAnalyticsRef.current;
+ if (cb) cb(event, payload);
+ });
+ });
+ } catch (error) {
+ console.error('Error initializing IVLabsPlayer:', error);
+ }
+ return () => {
+ if (playerRef.current) {
+ playerRef.current.destroy();
+ playerRef.current = null;
+ }
+ };
+ }, [videoUrl]);Additions required outside the selected range:
// Keep the latest analytics callback without retriggering init effect
const onAnalyticsRef = useRef<typeof onAnalyticsEvent>(onAnalyticsEvent);
useEffect(() => {
onAnalyticsRef.current = onAnalyticsEvent;
}, [onAnalyticsEvent]);Rationale:
- Prevents unnecessary player churn.
- Ensures analytics handler updates without re-instantiation.
Also applies to: 73-87, 99-99
🤖 Prompt for AI Agents
In src/index.tsx around lines 62 to 72, the player is being re-instantiated on
every render due to restOptions and onAnalyticsEvent being dependencies in the
effect, causing unnecessary teardown and flicker. To fix this, modify the effect
to only initialize and destroy the player when videoUrl changes. Additionally,
create a ref outside the effect to hold the latest onAnalyticsEvent callback and
update it via a separate useEffect, so the player uses the current callback
without needing to be recreated. Apply similar changes to the effect blocks at
lines 73-87 and line 99.
| useEffect(() => { | ||
| if (playerRef.current && cues) { | ||
| playerRef.current.loadCues(cues); | ||
| } | ||
| }, [cues]); | ||
|
|
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.
🛠️ Refactor suggestion
Ensure cues reload after player re-initialization.
If the player is recreated (e.g., when videoUrl changes), cues won’t reload unless this effect re-runs. Include videoUrl.
- }, [cues]);
+ }, [cues, videoUrl]);📝 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.
| useEffect(() => { | |
| if (playerRef.current && cues) { | |
| playerRef.current.loadCues(cues); | |
| } | |
| }, [cues]); | |
| useEffect(() => { | |
| if (playerRef.current && cues) { | |
| playerRef.current.loadCues(cues); | |
| } | |
| }, [cues, videoUrl]); |
🤖 Prompt for AI Agents
In src/index.tsx around lines 101 to 106, the useEffect hook that loads cues
only depends on cues, so if the player is re-initialized due to a change in
videoUrl, the cues won't reload. Add videoUrl to the dependency array of this
useEffect to ensure cues reload whenever the player is recreated or cues change.
| useEffect(() => { | ||
| if (playerRef.current && translations) { | ||
| const locale = restOptions.locale || 'en'; | ||
| playerRef.current.loadTranslations(locale, translations); | ||
| } | ||
| }, [translations, restOptions.locale]); |
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.
🛠️ Refactor suggestion
Ensure translations reload after player re-initialization and locale changes.
Similarly, reload translations when the player is recreated.
- }, [translations, restOptions.locale]);
+ }, [translations, restOptions.locale, videoUrl]);🤖 Prompt for AI Agents
In src/index.tsx around lines 107 to 112, the current useEffect reloads
translations only when translations or locale change, but it does not handle
reloading when the player is re-initialized. Modify the effect to also depend on
the player instance or a relevant state that changes when the player is
recreated, so that translations reload whenever the player is re-initialized or
the locale changes.
| import pkg from './package.json'; | ||
|
|
||
| export default defineConfig({ | ||
| export default defineConfig((options) => ({ |
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.
💡 Verification agent
🧩 Analysis chain
JSON import in config may require TS settings; provide portable fallback.
Importing package.json directly relies on resolveJsonModule (and compatible module resolution). If your env doesn’t guarantee this, prefer createRequire to avoid config-time import issues.
Verification script (checks tsconfig flags):
Optional portable change:
-import pkg from './package.json';
+// Portable JSON access without relying on resolveJsonModule
+import { createRequire } from 'node:module';
+const require = createRequire(import.meta.url);
+const pkg = require('./package.json') as {
+ name: string;
+ version: string;
+ author?: string;
+ license?: string;
+};Nit: consider enabling sourcemaps for easier debugging:
- clean: true,
+ clean: true,
+ sourcemap: true,Also applies to: 7-11, 13-13, 19-22, 23-23
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -t f 'tsconfig*.json' | while read -r f; do
echo "== $f =="
jq -r '.compilerOptions | {module, moduleResolution, resolveJsonModule, esModuleInterop}' "$f"
doneLength of output: 449
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# For each tsconfig*.json, print key compilerOptions fields
fd -t f 'tsconfig*.json' | while read -r f; do
echo "== $f =="
module=$(jq -r '.compilerOptions.module // "undefined"' "$f")
moduleResolution=$(jq -r '.compilerOptions.moduleResolution // "undefined"' "$f")
resolveJsonModule=$(jq -r '.compilerOptions.resolveJsonModule // false' "$f")
esModuleInterop=$(jq -r '.compilerOptions.esModuleInterop // false' "$f")
echo "module: $module"
echo "moduleResolution: $moduleResolution"
echo "resolveJsonModule: $resolveJsonModule"
echo "esModuleInterop: $esModuleInterop"
doneLength of output: 697
Enable portable JSON import in tsup.config.ts
Your tsconfig.json currently has resolveJsonModule: false, so the direct import pkg from './package.json' will fail under TypeScript compilation. To ensure your config loads package metadata reliably, switch to a Node‐native fallback or enable the compiler flag:
Locations to update:
- tsup.config.ts (around line 4)
- tsup.config.ts (config object, around
clean: true)
Suggested changes:
--- tsup.config.ts
@@ -1,7 +1,12
-import pkg from './package.json';
+// Portable JSON access without relying on resolveJsonModule
+import { createRequire } from 'node:module';
+const require = createRequire(import.meta.url);
+const pkg = require('./package.json') as {
+ name: string;
+ version: string;
+ author?: string;
+ license?: string;
+};
export default defineConfig((options) => ({
clean: true,
+ // Easier debugging of generated bundles
+ sourcemap: true,
// …
}));Alternatively, you can enable JSON imports by setting "resolveJsonModule": true in your tsconfig.json under compilerOptions.
📝 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.
| export default defineConfig((options) => ({ | |
| // Portable JSON access without relying on resolveJsonModule | |
| import { createRequire } from 'node:module'; | |
| const require = createRequire(import.meta.url); | |
| const pkg = require('./package.json') as { | |
| name: string; | |
| version: string; | |
| author?: string; | |
| license?: string; | |
| }; | |
| export default defineConfig((options) => ({ | |
| clean: true, | |
| // Easier debugging of generated bundles | |
| sourcemap: true, | |
| // … | |
| })); |
🤖 Prompt for AI Agents
In tsup.config.ts around line 4 and near the config object with clean: true,
avoid directly importing JSON since tsconfig.json has resolveJsonModule set to
false. Instead, replace the JSON import with a Node-native method like using
fs.readFileSync and JSON.parse to load package metadata at runtime.
Alternatively, enable "resolveJsonModule": true in tsconfig.json under
compilerOptions to allow direct JSON imports.
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores
Documentation