chore: improve CI/CD and formatting #41
Conversation
- Added version bump workflow - Improved release workflow - Added prettier config - Added eslint config - Updated changelog formatting
WalkthroughAdds manual trigger to release workflow, enhances tag-after-merge workflow to build release notes body, introduces ESLint/Prettier configs and scripts, formats documentation and examples, and significantly refactors src/index.ts to a strongly-typed Vue component with props-driven initialization, analytics forwarding, lifecycle management, watchers, and optional external target element handling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Parent
participant IV as InteractiveVideo (Vue)
participant DOM as DOM Container
participant Player as IVLabsPlayer
Parent->>IV: Mount with props (videoUrl, cues?, translations?, onAnalyticsEvent?, targetElementId?)
Note over IV: onMounted → nextTick
alt targetElementId provided
IV-->>DOM: Use existing element ID
else no targetElementId
IV->>DOM: Render div with generated ID
end
IV->>Player: new IVLabsPlayer(config merged from props + attrs)
opt cues provided
IV->>Player: loadCues(cues)
end
opt translations provided
IV->>Player: loadTranslations(translations)
end
opt onAnalyticsEvent provided
Player-->>IV: emit AnalyticsEvent(event, payload)
IV-->>Parent: onAnalyticsEvent(event, payload)
end
rect rgba(200, 230, 255, 0.3)
note right of IV: Watchers (deep)
IV->>Player: on cues change → loadCues(newCues)
IV->>Player: on translations change → loadTranslations(newTranslations)
end
Parent--x IV: Unmount
IV->>Player: destroy()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
DEVELOPER.md (1)
70-74: Docs outdated: linting scripts now exist.This section claims no linting script exists, but package.json adds lint/format scripts.
Apply this diff to align with current tooling:
-This project uses TypeScript. Ensure your code adheres to the existing style and type conventions. While there isn't an explicit linting script defined in `package.json`, it's good practice to follow consistent formatting and coding standards. +This project uses TypeScript, ESLint, and Prettier. Ensure your code adheres to the existing style and type conventions. + +Available scripts: +- `pnpm lint` – run ESLint +- `pnpm lint --fix` – auto-fix lint issues +- `pnpm format` – format with Prettier + +See `eslint.config.mjs` and `.prettierrc.json` for rules.CONTRIBUTING.md (1)
23-23: Fill in the Code of Conduct contact.Placeholder text remains; this blocks effective reporting.
Suggested fix:
-This project adheres to the [CODE_OF_CONDUCT.md](CODE_OF_CONDUCT.md). By participating, you are expected to uphold this code. Please report unacceptable behavior to [insert contact email or method here, e.g., `support@example.com`]. +This project adheres to the [CODE_OF_CONDUCT.md](CODE_OF_CONDUCT.md). By participating, you are expected to uphold this code. Please report unacceptable behavior to support@interactive-video-labs.com.README.md (1)
9-9: Fix broken downloads badge (shields.io path).
Usedm(monthly) ordt(total) instead ofd18m.- <img src="https://img.shields.io/npm/d18m/@interactive-video-labs/vue?style=flat-square" alt="NPM Downloads" /> + <img src="https://img.shields.io/npm/dm/@interactive-video-labs/vue?style=flat-square" alt="NPM Downloads (monthly)" />.github/workflows/tag-after-merge.yml (1)
64-70: CHANGELOG extraction regex misses headings without trailing space.
[[:space:]]requires a space after the version; lines like## v1.2.3won’t match. Use boundary-or-EOL.- awk "/^## v${{ env.version }}[[:space:]]/{flag=1; next} /^## /{flag=0} flag" CHANGELOG.md > RELEASE_NOTES.md + awk -v ver="v${{ env.version }}" ' + $0 ~ "^## " ver "($|[[:space:]])" {flag=1; next} + /^## / && flag {flag=0} + flag + ' CHANGELOG.md > RELEASE_NOTES.md
🧹 Nitpick comments (14)
package.json (2)
24-27: Avoid running lint --fix/format in prepare; fix trailing space.prepare runs on install (including consumers and CI) and can mutate the working tree unexpectedly. Keep prepare for build-only; run lint/format in CI or locally.
Apply:
- "lint": "eslint . --ext .ts", - "format": "prettier --write .", - "prepare": "pnpm lint --fix && pnpm format && pnpm build " + "lint": "eslint . --ext .ts", + "format": "prettier --write .", + "prepare": "pnpm build", + "prepublishOnly": "pnpm build"Optional additions for CI:
+ "lint:check": "eslint .", + "format:check": "prettier --check .",
24-24: Broaden lint extensions (optional).If you want ESLint to cover .mjs/.mts/.cts too:
- "lint": "eslint . --ext .ts", + "lint": "eslint . --ext .ts,.mts,.cts,.mjs",eslint.config.mjs (3)
34-37: Scope the TS parser to TS files onlyUsing the TS parser for all files can mis-parse plain JS. Add a files matcher so only TS/TSX use tsParser.
{ - extends: compat.extends( + files: ['**/*.{ts,tsx}'], + extends: compat.extends( 'eslint:recommended', 'plugin:@typescript-eslint/recommended', 'prettier', ), plugins: { '@typescript-eslint': typescriptEslint, prettier, }, languageOptions: { parser: tsParser, },
38-41: Avoid double-applying @typescript-eslint recommended rulesYou already extend the recommended config via compat. Spreading
typescriptEslint.configs.recommended.rulesduplicates rules and can cause drift.rules: { - ...typescriptEslint.configs.recommended.rules, 'prettier/prettier': 'error', },
23-33: Optional: Remove eslint-plugin-prettier for faster linting
Running Prettier as an ESLint rule adds overhead; you can extend'prettier'and run Prettier via CLI (or a git hook) instead. Your current versions are compatible (ESLint 9.34.0 & eslint-plugin-prettier 5.5.4), so keeping the plugin is safe if you prefer inline formatting.src/index.ts (4)
92-106: SSR/hydration-safety of random IDs
Math.random()in setup can cause server/client ID mismatch under SSR. Prefer a deterministic client-only ID or requiretargetElementIdin SSR.Example:
- Generate the random ID in
onMountedwhentargetElementIdis absent, then trigger init after it’s set.- Or use a stable prefix and an incrementing counter kept in module scope.
151-157: Initialization timing and external targetsIf
targetElementIdpoints to an element mounted by a parent later than yournextTick, initialization will fail. Consider a short retry/backoff or a MutationObserver for a brief window before giving up.
168-176: Handle clearing cues/translations as wellWatchers only load when new values exist. If these become
undefinedor empty, decide whether to clear them via a supported API (e.g.,loadCues([])or aclear*method).
85-113: Consider reacting to videoUrl/locale changesIf
videoUrlorlocalechange, the player isn’t updated. Add watchers (or a re-init path) if the core player supports it.+ watch(() => props.videoUrl, (url, prev) => { + if (playerRef.value && url && url !== prev && playerRef.value.loadVideo) { + // @ts-expect-error: depends on IVLabs API + playerRef.value.loadVideo(url); + } + }); + + watch(() => props.locale, (loc, prev) => { + if (playerRef.value && loc && props.translations) { + playerRef.value.loadTranslations(loc, props.translations); + } + });test/InteractiveVideo.test.ts (1)
39-53: Strengthen assertions (optional)Also assert constructor args to catch regressions (element ID and config shape).
expect(IVLabsPlayer).toHaveBeenCalledTimes(1); + expect((IVLabsPlayer as any).mock.calls[0][0]).toMatch(/^ivlabs-player-/); + expect((IVLabsPlayer as any).mock.calls[0][1]).toEqual( + expect.objectContaining({ videoUrl: 'https://example.com/video.mp4' }), + );examples/ExampleInteractiveVideo.vue (1)
15-17: Docs vs. example mismatch on analytics forwarding. Ensure both event and prop APIs are supported or align usage.
The example listens to@analytics-event, while README shows:on-analytics-event. Confirm both are valid or update one for consistency.Also applies to: 34-35
.github/workflows/release.yml (2)
8-11: Enable provenance (optional) by granting OIDC and publishing with--provenance.
Improves supply-chain security for npm.permissions: contents: read packages: write + id-token: write- - name: 🚀 Publish to npmjs - run: pnpm publish --access public --no-git-checks --registry=https://registry.npmjs.org/ + - name: 🚀 Publish to npmjs (with provenance) + run: pnpm publish --access public --no-git-checks --provenance --registry=https://registry.npmjs.org/ env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
27-31: Minor: Cache determinism.
Addcache-dependency-path: pnpm-lock.yamlfor more reliable caching.- name: 🔧 Setup Node.js uses: actions/setup-node@v5 with: node-version: 22 cache: 'pnpm' + cache-dependency-path: pnpm-lock.yaml.github/workflows/tag-after-merge.yml (1)
56-63: Guard against existing tag.
Avoid failures when the tag already exists.- name: Create and push git tag if: env.release_type != 'patch' run: | git config user.name "github-actions[bot]" git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git tag v${{ env.version }} - git push origin v${{ env.version }} + if git rev-parse -q --verify "refs/tags/v${{ env.version }}"; then + echo "Tag v${{ env.version }} already exists. Skipping." + else + git tag v${{ env.version }} + git push origin v${{ env.version }} + fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
examples/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
.github/workflows/release.yml(1 hunks).github/workflows/tag-after-merge.yml(2 hunks).github/workflows/version-bump.yml(0 hunks).prettierrc.json(1 hunks)CHANGELOG.md(1 hunks)CODE_OF_CONDUCT.md(2 hunks)CONTRIBUTING.md(3 hunks)DEVELOPER.md(2 hunks)README.md(4 hunks)eslint.config.mjs(1 hunks)examples/ExampleInteractiveVideo.vue(2 hunks)examples/index.html(1 hunks)package.json(2 hunks)renovate.json(1 hunks)src/index.ts(1 hunks)test/InteractiveVideo.test.ts(1 hunks)tsconfig.json(1 hunks)tsup.config.ts(2 hunks)vitest.config.ts(0 hunks)
💤 Files with no reviewable changes (2)
- .github/workflows/version-bump.yml
- vitest.config.ts
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~91-~91: To form a complete sentence, be sure to include a subject.
Context: ...y of cue points for interactive events. Can be updated dynamically. | | `translatio...
(MISSING_IT_THERE)
CONTRIBUTING.md
[grammar] ~35-~35: There might be a mistake here.
Context: .... - Screenshots or videos if applicable. - Your environment (Vue version, browser, ...
(QB_NEW_EN)
CODE_OF_CONDUCT.md
[grammar] ~13-~13: There might be a mistake here.
Context: ...empathy and kindness toward other people - Being respectful of differing opinions, ...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ...ng opinions, viewpoints, and experiences - Giving and gracefully accepting construc...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...acefully accepting constructive feedback - Accepting responsibility and apologizing...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...stakes, and learning from the experience - Focusing on what is best not just for us...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...sexual attention or advances of any kind - Trolling, insulting or derogatory commen...
(QB_NEW_EN)
[style] ~22-~22: Try using a synonym here to strengthen your wording.
Context: ...ind - Trolling, insulting or derogatory comments, and personal or political attacks - Pu...
(COMMENT_REMARK)
[grammar] ~22-~22: There might be a mistake here.
Context: ...ments, and personal or political attacks - Public or private harassment - Publishin...
(QB_NEW_EN)
[grammar] ~23-~23: There might be a mistake here.
Context: ...l attacks - Public or private harassment - Publishing others' private information, ...
(QB_NEW_EN)
[grammar] ~24-~24: There might be a mistake here.
Context: ...dress, without their explicit permission - Other conduct which can reasonably be co...
(QB_NEW_EN)
🔇 Additional comments (12)
.prettierrc.json (1)
2-6: LGTM: sensible Prettier defaults.These baseline rules look good for a TypeScript/Vue repo.
renovate.json (1)
3-3: LGTM: formatting-only change.No behavioral impact.
tsconfig.json (1)
18-18: LGTM: formatting-only on exclude list.No change in compiler behavior.
CHANGELOG.md (1)
14-24: LGTM: list marker normalization.Minor formatting improvement; content unchanged.
tsup.config.ts (1)
5-26: LGTM: formatting-only; build behavior unchanged.Entry, formats, dts, clean, and banner remain intact.
DEVELOPER.md (1)
10-10: Formatting-only changes LGTM.Consistent with new Prettier config; no content issues.
Also applies to: 30-40, 42-58
CONTRIBUTING.md (1)
31-36: Formatting changes LGTM.Bullet and code block normalization improves readability.
Also applies to: 42-44, 71-73, 85-89, 93-96
CODE_OF_CONDUCT.md (1)
13-17: Formatting-only changes LGTM.List marker normalization and reference label tweak are correct; no semantic changes.
Also applies to: 21-25, 70-70
examples/index.html (1)
1-1: HTML5 formatting LGTM.Lowercase doctype and self-closing meta tags are valid; no behavioral change.
Also applies to: 3-5, 7-8, 11-11
src/index.ts (1)
120-137: Validate event names against the core APIHard-coded
eventsToRegistermust matchAnalyticsEvent. Confirm against the core package to avoid silent no-ops.test/InteractiveVideo.test.ts (1)
96-99: LGTM: robustly retrieving the registered callbackThe multi-line predicate is clearer and keeps the same behavior.
README.md (1)
141-147: Verify exposed instance API name (playerRef).
Confirm the component actually exposesplayerRef; if not, document the correct property.
| "singleQuote": true, | ||
| "printWidth": 100, | ||
| "tabWidth": 2, | ||
| "ignores": ["node_modules", "dist"] |
There was a problem hiding this comment.
Replace unsupported "ignores" with .prettierignore.
Prettier config doesn’t support an "ignores" key; it’ll be ignored. Use a .prettierignore file instead to exclude paths like dist.
Apply this diff to the config and add an ignore file:
"tabWidth": 2,
- "ignores": ["node_modules", "dist"]
+ // moved to .prettierignore.new file: .prettierignore
node_modules
dist
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
.prettierrc.json around line 7: the config contains an unsupported "ignores" key
which Prettier ignores; remove the "ignores" entry from .prettierrc.json and
instead create a new .prettierignore file at the repo root containing the
ignored paths (node_modules and dist) so Prettier will properly exclude them.
| videoUrl="http://commondatastorage.googleapis.com/gtv-videos-bucket/sample/BigBuckBunny.mp4" | ||
| :autoplay="false" |
There was a problem hiding this comment.
Switch sample video URLs to HTTPS to avoid mixed-content blocking on HTTPS sites.
- videoUrl="http://commondatastorage.googleapis.com/gtv-videos-bucket/sample/BigBuckBunny.mp4"
+ videoUrl="https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/BigBuckBunny.mp4"
...
- videoUrl="http://commondatastorage.googleapis.com/gtv-videos-bucket/sample/BigBuckBunny.mp4"
+ videoUrl="https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/BigBuckBunny.mp4"Also applies to: 30-31
🤖 Prompt for AI Agents
In examples/ExampleInteractiveVideo.vue around lines 11-12 (and also lines
30-31), the sample video URL uses HTTP which can be blocked as mixed-content on
HTTPS sites; replace the http:// URL with its HTTPS equivalent (e.g.,
https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/BigBuckBunny.mp4)
in both locations so the demo works on secure pages.
| "prettier": "^3.6.2", | ||
| "@eslint/eslintrc": "^3.3.1", | ||
| "@eslint/js": "^9.34.0", | ||
| "@typescript-eslint/eslint-plugin": "^8.42.0", | ||
| "@typescript-eslint/parser": "^8.42.0", | ||
| "eslint": "^9.34.0", | ||
| "eslint-config-prettier": "^10.1.8", | ||
| "eslint-plugin-prettier": "^5.5.4", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Tooling versions look consistent. Consider declaring Node engine.
ESLint 9 requires Node >= 18.18. Declare engine to prevent CI/local drift.
Run to verify Node versions used in workflows and presence of config files:
If not already present, consider adding:
+ "engines": {
+ "node": ">=18.18"
+ },🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Node versions referenced in workflows:"
rg -nC2 -g ".github/workflows/*.yml" 'setup-node@|node-version:'
echo -e "\nCheck for eslint/prettier config files:"
fd -a 'eslint.config.mjs' || true
fd -a '.prettierrc*' || true
fd -a '.prettierignore' || true
echo -e "\nLocal Node version (if set via .nvmrc/.tool-versions):"
fd -a '.nvmrc' -x sh -c 'echo ".nvmrc:"; cat {}'
fd -a '.tool-versions' -x sh -c 'echo ".tool-versions:"; cat {}'Length of output: 322
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Workflow files ==="
find . -type f -path "./.github/workflows/*.yml" -print || true
echo -e "\n=== Node versions in workflows ==="
grep -R -n -C2 'node-version:' .github/workflows || true
grep -R -n -C2 'setup-node@' .github/workflows || true
echo -e "\n=== ESLint/Prettier config files ==="
find . -type f \( -name 'eslint.config.mjs' -o -name '.prettierrc*' -o -name '.prettierignore' \) -print || true
echo -e "\n=== Local Node version files ==="
find . -type f \( -name '.nvmrc' -o -name '.tool-versions' \) -print -exec sh -c 'echo "Contents of {}:"; cat {}' \; || trueLength of output: 2213
Add Node engine constraint
Workflows run Node 22 but local environments aren’t pinned—add to package.json:
"engines": {
"node": ">=18.18"
}to align CI/local versions and satisfy ESLint 9’s Node ≥18.18 requirement.
🤖 Prompt for AI Agents
In package.json around lines 58 to 65, add a top-level "engines" field
specifying the Node version constraint (e.g., "node": ">=18.18") so local dev
environments are pinned to the same minimum Node version required by ESLint 9
and CI; insert the "engines" object at the root of package.json (near other
top-level metadata) with the specified node range and save the file.
| | Prop | Type | Required | Default | Description | | ||
| | ------------------ | ------------------------------------------------------------- | -------- | ------- | -------------------------------------------------------------------------- | | ||
| | `videoUrl` | `string` | `true` | - | The URL of the video to be loaded. | | ||
| | `cues` | `CuePoint[]` | `false` | `[]` | An array of cue points for interactive events. Can be updated dynamically. | | ||
| | `translations` | `Translations` | `false` | `{}` | An object containing translations for the player UI. | | ||
| | `onAnalyticsEvent` | `(event: AnalyticsEvent, payload?: AnalyticsPayload) => void` | `false` | - | Callback function for analytics events emitted by the player. | | ||
| | `autoplay` | `boolean` | `false` | `false` | Whether the video should start playing automatically. | | ||
| | `loop` | `boolean` | `false` | `false` | Whether the video should loop. | | ||
| | `locale` | `string` | `false` | `'en'` | The locale to be used for the player (e.g., 'en', 'es'). | |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Props table: add missing targetElementId and fix cues description grammar.
Also matches the example component usage.
-| `cues` | `CuePoint[]` | `false` | `[]` | An array of cue points for interactive events. Can be updated dynamically. |
+| `cues` | `CuePoint[]` | `false` | `[]` | An array of cue points for interactive events that can be updated dynamically. |
| `translations` | `Translations` | `false` | `{}` | An object containing translations for the player UI. |
| `onAnalyticsEvent` | `(event: AnalyticsEvent, payload?: AnalyticsPayload) => void` | `false` | - | Callback function for analytics events emitted by the player. |
| `autoplay` | `boolean` | `false` | `false` | Whether the video should start playing automatically. |
| `loop` | `boolean` | `false` | `false` | Whether the video should loop. |
| `locale` | `string` | `false` | `'en'` | The locale to be used for the player (e.g., 'en', 'es'). |
+| `targetElementId` | `string` | `false` | - | ID of an existing DOM element to mount the player into (optional). |📝 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.
| | Prop | Type | Required | Default | Description | | |
| | ------------------ | ------------------------------------------------------------- | -------- | ------- | -------------------------------------------------------------------------- | | |
| | `videoUrl` | `string` | `true` | - | The URL of the video to be loaded. | | |
| | `cues` | `CuePoint[]` | `false` | `[]` | An array of cue points for interactive events. Can be updated dynamically. | | |
| | `translations` | `Translations` | `false` | `{}` | An object containing translations for the player UI. | | |
| | `onAnalyticsEvent` | `(event: AnalyticsEvent, payload?: AnalyticsPayload) => void` | `false` | - | Callback function for analytics events emitted by the player. | | |
| | `autoplay` | `boolean` | `false` | `false` | Whether the video should start playing automatically. | | |
| | `loop` | `boolean` | `false` | `false` | Whether the video should loop. | | |
| | `locale` | `string` | `false` | `'en'` | The locale to be used for the player (e.g., 'en', 'es'). | | |
| | Prop | Type | Required | Default | Description | | |
| | ------------------ | ------------------------------------------------------------- | -------- | ------- | -------------------------------------------------------------------------- | | |
| | `videoUrl` | `string` | `true` | - | The URL of the video to be loaded. | | |
| | `cues` | `CuePoint[]` | `false` | `[]` | An array of cue points for interactive events that can be updated dynamically. | | |
| | `translations` | `Translations` | `false` | `{}` | An object containing translations for the player UI. | | |
| | `onAnalyticsEvent` | `(event: AnalyticsEvent, payload?: AnalyticsPayload) => void` | `false` | - | Callback function for analytics events emitted by the player. | | |
| | `autoplay` | `boolean` | `false` | `false` | Whether the video should start playing automatically. | | |
| | `loop` | `boolean` | `false` | `false` | Whether the video should loop. | | |
| | `locale` | `string` | `false` | `'en'` | The locale to be used for the player (e.g., 'en', 'es'). | | |
| | `targetElementId` | `string` | `false` | - | ID of an existing DOM element to mount the player into (optional). | |
🧰 Tools
🪛 LanguageTool
[style] ~91-~91: To form a complete sentence, be sure to include a subject.
Context: ...y of cue points for interactive events. Can be updated dynamically. | | `translatio...
(MISSING_IT_THERE)
🤖 Prompt for AI Agents
In README.md around lines 88 to 96, the props table is missing the
targetElementId prop and the cues description has awkward grammar; add a new row
for `targetElementId` (type `string`, required `false`, default `undefined` or
`-`, description like "The id of the DOM element where the player will be
mounted.") and update the `cues` description to grammatically read something
like "An array of cue points for interactive events; can be updated
dynamically." Make sure the new prop matches the example component usage and
keep formatting consistent with the existing table.
| setup(props, { attrs, expose }) { | ||
| const containerRef = ref<HTMLDivElement | null>(null); | ||
| const playerRef = ref<IVLabsPlayer | null>(null); | ||
|
|
||
| // Determine the ID to be used by the IVLabsPlayer. | ||
| // If targetElementId is provided, use it directly. | ||
| // Otherwise, generate a unique ID for the div rendered by this component. | ||
| const playerTargetId = | ||
| props.targetElementId || `ivlabs-player-${Math.random().toString(36).substr(2, 9)}`; | ||
|
|
||
| const initializePlayer = () => { | ||
| if (playerRef.value) { | ||
| return; // Player already initialized | ||
| } | ||
|
|
||
| // Ensure the target element exists in the DOM before proceeding. | ||
| const targetElement = document.getElementById(playerTargetId); | ||
| if (!targetElement) { | ||
| console.error(`IVLabsPlayer target element with ID '${playerTargetId}' not found.`); | ||
| return; | ||
| } | ||
|
|
||
| const playerConfig: PlayerConfig = { | ||
| ...attrs, | ||
| videoUrl: props.videoUrl, | ||
| autoplay: props.autoplay, | ||
| loop: props.loop, | ||
| locale: props.locale, | ||
| }; |
There was a problem hiding this comment.
Don’t stuff $attrs into PlayerConfig; also $attrs aren’t forwarded to the root div
attrs may include DOM attrs (class, style, id, data-*) and Vue listeners; merging them into PlayerConfig is leaky and they won’t be rendered on the container since you don’t bind attrs in the VNode.
Apply:
- Stop spreading
attrsintoplayerConfig. - Manually forward
attrsto the rendered div. - Set
inheritAttrs: falseto avoid surprises.
export default defineComponent({
name: 'InteractiveVideo',
+ inheritAttrs: false,
props: {
@@
- const playerConfig: PlayerConfig = {
- ...attrs,
- videoUrl: props.videoUrl,
- autoplay: props.autoplay,
- loop: props.loop,
- locale: props.locale,
- };
+ const playerConfig: PlayerConfig = {
+ videoUrl: props.videoUrl,
+ autoplay: props.autoplay,
+ loop: props.loop,
+ locale: props.locale,
+ };
@@
- return h('div', {
- ref: containerRef,
- id: playerTargetId, // Use the generated ID for this div
- style: { width: '100%', height: 'auto' },
- 'data-testid': 'interactive-video-container',
- });
+ return h('div', {
+ ...attrs,
+ ref: containerRef,
+ id: playerTargetId,
+ style: { width: '100%', height: 'auto' },
+ 'data-testid': 'interactive-video-container',
+ } as any);📝 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.
| setup(props, { attrs, expose }) { | |
| const containerRef = ref<HTMLDivElement | null>(null); | |
| const playerRef = ref<IVLabsPlayer | null>(null); | |
| // Determine the ID to be used by the IVLabsPlayer. | |
| // If targetElementId is provided, use it directly. | |
| // Otherwise, generate a unique ID for the div rendered by this component. | |
| const playerTargetId = | |
| props.targetElementId || `ivlabs-player-${Math.random().toString(36).substr(2, 9)}`; | |
| const initializePlayer = () => { | |
| if (playerRef.value) { | |
| return; // Player already initialized | |
| } | |
| // Ensure the target element exists in the DOM before proceeding. | |
| const targetElement = document.getElementById(playerTargetId); | |
| if (!targetElement) { | |
| console.error(`IVLabsPlayer target element with ID '${playerTargetId}' not found.`); | |
| return; | |
| } | |
| const playerConfig: PlayerConfig = { | |
| ...attrs, | |
| videoUrl: props.videoUrl, | |
| autoplay: props.autoplay, | |
| loop: props.loop, | |
| locale: props.locale, | |
| }; | |
| export default defineComponent({ | |
| name: 'InteractiveVideo', | |
| inheritAttrs: false, | |
| props: { | |
| targetElementId: String, | |
| videoUrl: String, | |
| autoplay: Boolean, | |
| loop: Boolean, | |
| locale: String, | |
| }, | |
| setup(props, { attrs, expose }) { | |
| const containerRef = ref<HTMLDivElement | null>(null); | |
| const playerRef = ref<IVLabsPlayer | null>(null); | |
| // Determine the ID to be used by the IVLabsPlayer. | |
| // If targetElementId is provided, use it directly. | |
| // Otherwise, generate a unique ID for the div rendered by this component. | |
| const playerTargetId = | |
| props.targetElementId || | |
| `ivlabs-player-${Math.random().toString(36).substr(2, 9)}`; | |
| const initializePlayer = () => { | |
| if (playerRef.value) { | |
| return; // Player already initialized | |
| } | |
| // Ensure the target element exists in the DOM before proceeding. | |
| const targetElement = document.getElementById(playerTargetId); | |
| if (!targetElement) { | |
| console.error( | |
| `IVLabsPlayer target element with ID '${playerTargetId}' not found.` | |
| ); | |
| return; | |
| } | |
| const playerConfig: PlayerConfig = { | |
| videoUrl: props.videoUrl, | |
| autoplay: props.autoplay, | |
| loop: props.loop, | |
| locale: props.locale, | |
| }; | |
| // ...initialize your player with playerConfig... | |
| }; | |
| return () => | |
| h( | |
| 'div', | |
| { | |
| ...attrs, | |
| ref: containerRef, | |
| id: playerTargetId, | |
| style: { width: '100%', height: 'auto' }, | |
| 'data-testid': 'interactive-video-container', | |
| } as any | |
| ); | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In src/index.ts around lines 85 to 113, the setup is incorrectly spreading attrs
into the PlayerConfig and not forwarding attrs to the rendered root div; remove
the spread of attrs from playerConfig, add inheritAttrs: false at the component
level, and ensure the template/render function binds attrs (e.g., v-bind="attrs"
or ...attrs on the root div) so DOM attributes and listeners are forwarded to
the container element instead of leaking into the player config.
Summary by CodeRabbit
New Features
Documentation
Chores