chore: support code lint and commit lint #8
Conversation
WalkthroughThis update introduces configuration and tooling for code linting, formatting, and commit message validation. It adds Husky Git hooks, ESLint and Prettier configurations, and commitlint support. Minor formatting and stylistic changes are made across source, test, and configuration files, with no functional modifications to application logic or public APIs. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Git
participant Husky Hooks
participant Lint/Commitlint Tools
Developer->>Git: git commit
Git->>Husky Hooks: Trigger pre-commit
Husky Hooks->>Lint/Commitlint Tools: Run pnpm run lint
Lint/Commitlint Tools-->>Husky Hooks: Lint passes/fails
Husky Hooks->>Git: Continue if lint passes
Git->>Husky Hooks: Trigger commit-msg
Husky Hooks->>Lint/Commitlint Tools: Run pnpm run commitlint
Lint/Commitlint Tools-->>Husky Hooks: Commit message valid/invalid
Husky Hooks->>Git: Continue if commitlint passes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 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. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (8)
.prettierrc (1)
1-6: Prettier defaults are reasonable and consistentGood baseline config. Consider adding a .prettierignore (node_modules, dist, coverage) and optionally
endOfLine: "lf"andprintWidthfor consistency across OS/editors.Example .prettierignore:
- node_modules
- dist
- build
- coverage
- .husky
src/index.tsx (2)
75-95: Optional: DRY up event listener wiringYou can map events to reduce repetition.
Example:
const events: AnalyticsEvent[] = [ 'PLAYER_LOADED', 'VIDEO_STARTED', 'VIDEO_PAUSED', 'VIDEO_ENDED', 'CUE_TRIGGERED', 'INTERACTION_COMPLETED', 'ERROR', ]; events.forEach(evt => player.on(evt, (payload?: AnalyticsPayload) => onAnalyticsEvent?.(evt, payload)) );
119-119: Dependency array: restOptions identity can cause frequent re-initsSpreading props into
restOptionsmay change identity each render, triggering effect cleanup/init. If that’s undesired, memoize or list specific dependencies you want to react to.Possible approach:
- Use
useMemoto stabilize options.- Or destructure and list specific option keys in the dependency array.
test/setupTests.ts (1)
1-2: Remove commented import; configure Vitest globals for types insteadCommented-out imports tend to rot. If you need
expecttypes, prefer enabling Vitest globals in TS config rather than importing in setup.Recommended change:
-// import { expect } from 'vitest'; import '@testing-library/jest-dom/vitest';And ensure TS config includes Vitest globals (e.g., in tsconfig.json or tsconfig.test.json):
{ "compilerOptions": { "types": ["vitest/globals", "@testing-library/jest-dom"] } }examples/index.tsx (1)
11-26: External demo URL updated — consider making it configurableFine for examples, but hardcoding external assets can break demos if the URL changes. Consider a constant or env-based override.
Example:
- <InteractiveVideo - videoUrl="https://interactive-video-labs.github.io/assets/sample-interaction-1.mp4" + const DEMO_VIDEO_URL = import.meta.env.VITE_DEMO_VIDEO_URL ?? "https://interactive-video-labs.github.io/assets/sample-interaction-1.mp4"; + <InteractiveVideo + videoUrl={DEMO_VIDEO_URL} autoplay={true}tsup.config.ts (1)
10-15: Trim trailing whitespace-only line in banner stringMinor nit: the template literal ends with a line that contains only spaces. Not harmful, but it introduces stray whitespace in the emitted banner comment.
Apply:
- js: `/** + js: `/** * ${pkg.name} v${pkg.version} * Author: ${pkg.author} */ - `, +`,package.json (2)
16-21: Tighten lint-staged commandsRecommend caching ESLint and enforcing zero warnings on commit.
- "*.{js,jsx,ts,tsx}": [ - "eslint --fix", - "prettier --write --cache" - ] + "*.{js,jsx,ts,tsx}": [ + "eslint --fix --cache --max-warnings=0", + "prettier --write --cache" + ]
75-77: Streamline TypeScript ESLint deps (avoid duplication with the meta package)Since you’re using the typescript-eslint meta package in your flat config, you can drop the legacy plugin/parser entries to reduce version skew risk.
- "@typescript-eslint/eslint-plugin": "^8.39.0", - "@typescript-eslint/parser": "^8.39.0", - "typescript-eslint": "^8.39.0" + "typescript-eslint": "^8.39.0"Also applies to: 86-86
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
.husky/commit-msg(1 hunks).husky/pre-commit(1 hunks).prettierrc(1 hunks)commitlint.config.js(1 hunks)eslint.config.js(1 hunks)examples/index.html(1 hunks)examples/index.tsx(2 hunks)package.json(3 hunks)src/index.tsx(4 hunks)test/InteractiveVideo.test.tsx(3 hunks)test/setupTests.ts(1 hunks)tsconfig.json(1 hunks)tsup.config.ts(1 hunks)vitest.config.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
test/InteractiveVideo.test.tsx (1)
src/index.tsx (1)
InteractiveVideo(50-131)
🔇 Additional comments (7)
vitest.config.ts (1)
11-11: No-op formatting change looks goodNewline at EOF aligns with common linters and Prettier defaults. No functional impact.
tsconfig.json (1)
19-20: Confirm intentional exclusion of test files in tsconfig.json
TypeScript will skip any file or directory named"test"per the
"exclude": ["node_modules", "dist", "test"]setting intsconfig.json(lines 19–20). We currently have only one tsconfig and no test-specific config.• If you need TS type-checking for tests:
– Remove"test"fromexclude, or
– Adjustincludeto cover your test files, or
– Create atsconfig.test.jsonwith appropriateinclude/excludefor tests.• If it’s intentional to omit tests from type-checks/builds, no further changes are needed.
commitlint.config.js (1)
1-1: ESM Configuration ValidatedThe project’s package.json specifies
"type": "module", so using an ES module config (export default …) is correct. No changes required.src/index.tsx (2)
22-23: Minor improvement: replace deprecated substr with sliceGood change.
sliceavoids deprecatedsubstrusage.
71-71: Constructor target: verify IVLabsPlayer expects selector, element, or idYou’re passing the raw id (e.g., "ivlabs-player-xxxx"). If the constructor expects a CSS selector (common), it likely needs
#${id}. If it supports HTMLElement, prefer passingcontainerRef.current.Please confirm the expected type. If selector is required:
-const player = new IVLabsPlayer(uniqueIdRef.current, playerConfig); +const player = new IVLabsPlayer(`#${uniqueIdRef.current}`, playerConfig);If element is supported:
-const player = new IVLabsPlayer(uniqueIdRef.current, playerConfig); +const player = new IVLabsPlayer(containerRef.current, playerConfig);If docs aren’t at hand, I can help adapt based on the library signature.
Also applies to: 122-127
examples/index.html (1)
3-15: Formatting-only changes look goodtest/InteractiveVideo.test.tsx (1)
41-49: LGTM — formatting onlyReadability improvements without behavior changes. Timers/mocks usage remains intact.
Also applies to: 56-61, 80-83
| #!/usr/bin/env sh | ||
| pnpm run commitlint No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pass the commit message to commitlint and source husky.sh
Without --edit "$1", commitlint may not validate the message file. Also source husky.sh for environment setup.
Suggested hook:
#!/usr/bin/env sh
-pnpm run commitlint
+. "$(dirname -- "$0")/_/husky.sh"
+set -e
+pnpm exec commitlint --edit "$1"📝 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.
| #!/usr/bin/env sh | |
| pnpm run commitlint | |
| #!/usr/bin/env sh | |
| . "$(dirname -- "$0")/_/husky.sh" | |
| set -e | |
| pnpm exec commitlint --edit "$1" |
🤖 Prompt for AI Agents
In the .husky/commit-msg file at lines 1 to 2, the commitlint command does not
receive the commit message file as an argument, so it may not validate the
commit message properly. Modify the script to source the husky.sh script for
environment setup and run commitlint with the --edit "$1" option to pass the
commit message file for validation.
| import reactPlugin from 'eslint-plugin-react'; | ||
| import eslintPluginPrettierRecommended from 'eslint-plugin-prettier/recommended'; | ||
| import eslintConfigPrettier from 'eslint-config-prettier/flat'; | ||
| import { defineConfig } from 'eslint/config'; | ||
| import tseslint from 'typescript-eslint'; | ||
|
|
||
| export default defineConfig( | ||
| tseslint.configs.recommended, | ||
| reactPlugin.configs.flat.recommended, | ||
| reactPlugin.configs.flat['jsx-runtime'], | ||
| eslintPluginPrettierRecommended, | ||
| eslintConfigPrettier | ||
| ); |
There was a problem hiding this comment.
Invalid ESLint flat config composition (defineConfig usage and array/object mixing)
The current export won’t work as intended:
- defineConfig should be passed a single array/object, not multiple positional args.
- tseslint.configs.recommended is an array and must be spread into the config list.
Use a single array and spread TS configs:
-import reactPlugin from 'eslint-plugin-react';
-import eslintPluginPrettierRecommended from 'eslint-plugin-prettier/recommended';
-import eslintConfigPrettier from 'eslint-config-prettier/flat';
-import { defineConfig } from 'eslint/config';
-import tseslint from 'typescript-eslint';
-
-export default defineConfig(
- tseslint.configs.recommended,
- reactPlugin.configs.flat.recommended,
- reactPlugin.configs.flat['jsx-runtime'],
- eslintPluginPrettierRecommended,
- eslintConfigPrettier
-);
+import reactPlugin from 'eslint-plugin-react';
+import eslintPluginPrettierRecommended from 'eslint-plugin-prettier/recommended';
+import eslintConfigPrettier from 'eslint-config-prettier/flat';
+import { defineConfig } from 'eslint/config';
+import tseslint from 'typescript-eslint';
+
+export default defineConfig([
+ // TypeScript ESLint recommended (array)
+ ...tseslint.configs.recommended,
+ // React plugin (objects)
+ reactPlugin.configs.flat.recommended,
+ reactPlugin.configs.flat['jsx-runtime'],
+ // Prettier plugin + Prettier config (keep last)
+ eslintPluginPrettierRecommended,
+ eslintConfigPrettier,
+]);Optional: If you want auto React version detection, you can append a small config object:
{ settings: { react: { version: 'detect' } } }🤖 Prompt for AI Agents
In eslint.config.js lines 1 to 13, the defineConfig function is incorrectly
called with multiple positional arguments instead of a single array or object.
To fix this, wrap all configurations in a single array and spread the
tseslint.configs.recommended array into it. Optionally, add a config object with
settings for React version detection at the end of the array.
| "prepare": "husky", | ||
| "lint": "lint-staged", | ||
| "commitlint": "commitlint --edit", | ||
| "serve-examples": "pnpm build-examples && http-server examples -c-1", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify Husky hooks call the right scripts (commit-msg must pass the file to commitlint)
The scripts look good. Please confirm that:
- .husky/pre-commit invokes lint-staged (e.g., pnpm lint).
- .husky/commit-msg calls pnpm commitlint --edit "$1".
Run this script to verify hook presence and contents:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking Husky hooks..."
if [ ! -d ".husky" ]; then
echo "❌ .husky directory not found"; exit 1
fi
echo "== .husky/pre-commit =="
if [ -f ".husky/pre-commit" ]; then
sed -n '1,120p' .husky/pre-commit
echo
rg -n "lint-staged|pnpm lint" .husky/pre-commit || echo "⚠️ pre-commit may not run lint-staged"
else
echo "❌ .husky/pre-commit not found"
fi
echo
echo "== .husky/commit-msg =="
if [ -f ".husky/commit-msg" ]; then
sed -n '1,120p' .husky/commit-msg
echo
rg -n "commitlint\s+--edit\s+\$1|pnpm\s+commitlint\s+--edit\s+\$1" .husky/commit-msg || echo "⚠️ commit-msg may not pass the commit file to commitlint"
else
echo "❌ .husky/commit-msg not found"
fiLength of output: 780
Ensure Husky commit-msg hook passes the commit file to commitlint
The pre-commit hook is correctly invoking lint-staged via pnpm run lint. However, the commit-msg hook only runs pnpm run commitlint and doesn’t forward the $1 parameter, so commitlint never sees the commit message file. Please update .husky/commit-msg as shown:
# .husky/commit-msg
- pnpm run commitlint
+ pnpm run commitlint -- "$1"Or, if you prefer calling the CLI directly:
# .husky/commit-msg
- pnpm run commitlint
+ pnpm commitlint --edit "$1"This will ensure your commit messages are actually passed to commitlint for validation.
🤖 Prompt for AI Agents
In package.json around lines 30 to 33, the commit-msg hook does not pass the
commit message file parameter to commitlint, so commitlint cannot validate the
commit message. Update the .husky/commit-msg script to forward the $1 argument
to commitlint, either by modifying the script to run "pnpm run commitlint -- $1"
or by calling the commitlint CLI directly with the commit message file argument.
This ensures commitlint receives the commit message file for proper validation.
| "@commitlint/cli": "^19.8.1", | ||
| "@commitlint/config-conventional": "^19.8.1", | ||
| "@testing-library/jest-dom": "^6.6.4", | ||
| "@testing-library/react": "^16.0.0", | ||
| "@types/react": "^19.1.9", | ||
| "http-server": "^14.1.1" | ||
| "@typescript-eslint/eslint-plugin": "^8.39.0", | ||
| "@typescript-eslint/parser": "^8.39.0", | ||
| "eslint": "^9.33.0", | ||
| "eslint-config-prettier": "^10.1.8", | ||
| "eslint-plugin-prettier": "^5.5.4", | ||
| "eslint-plugin-react": "^7.37.5", | ||
| "exbuild": "^0.2.2", | ||
| "http-server": "^14.1.1", | ||
| "husky": "^9.1.7", | ||
| "lint-staged": "^16.1.5", | ||
| "prettier": "^3.6.2", | ||
| "typescript-eslint": "^8.39.0" | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix devDependency typo: use esbuild (not exbuild) to match your build script
The examples script uses esbuild, but devDependencies add exbuild. This will break pnpm run build-examples with pnpm because pnpm doesn’t link transitive bins to the root.
Replace exbuild with esbuild:
- "exbuild": "^0.2.2",
+ "esbuild": "^0.24.0",Note: Adjust the version if your CI constraints differ; any recent 0.2x should work. Ensure esbuild is a direct devDependency so its CLI is available at node_modules/.bin/esbuild under pnpm.
You can validate locally with:
- pnpm i
- pnpm esbuild --version
- pnpm run build-examples
🏁 Script executed:
#!/bin/bash
set -eo pipefail
# Display the "scripts" section of package.json to confirm build-examples usage
sed -n '1,200p' package.json | sed -n '/"scripts"/,/\}/p'
# Check for the typo "exbuild" in devDependencies
grep -R '"exbuild"' -n package.json || echo "No exbuild found outside snippet."
# Search the entire repo for any references to esbuild in code or scripts
grep -R 'esbuild' -n . || echo "No esbuild references found."Length of output: 266639
Fix typo in devDependencies: replace exbuild with esbuild
The build-examples script invokes esbuild, but you’ve added exbuild as a devDependency, so pnpm run build-examples will fail to find the CLI. Update your package.json devDependencies:
"devDependencies": {
- "exbuild": "^0.2.2",
+ "esbuild": "^0.25.0",
// …other deps
}Pick a recent 0.x version (your lockfile shows 0.25.8), then run:
- pnpm install
- pnpm esbuild --version
- pnpm run build-examples
📝 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.
| "@commitlint/cli": "^19.8.1", | |
| "@commitlint/config-conventional": "^19.8.1", | |
| "@testing-library/jest-dom": "^6.6.4", | |
| "@testing-library/react": "^16.0.0", | |
| "@types/react": "^19.1.9", | |
| "http-server": "^14.1.1" | |
| "@typescript-eslint/eslint-plugin": "^8.39.0", | |
| "@typescript-eslint/parser": "^8.39.0", | |
| "eslint": "^9.33.0", | |
| "eslint-config-prettier": "^10.1.8", | |
| "eslint-plugin-prettier": "^5.5.4", | |
| "eslint-plugin-react": "^7.37.5", | |
| "exbuild": "^0.2.2", | |
| "http-server": "^14.1.1", | |
| "husky": "^9.1.7", | |
| "lint-staged": "^16.1.5", | |
| "prettier": "^3.6.2", | |
| "typescript-eslint": "^8.39.0" | |
| } | |
| "devDependencies": { | |
| "@commitlint/cli": "^19.8.1", | |
| "@commitlint/config-conventional": "^19.8.1", | |
| "@testing-library/jest-dom": "^6.6.4", | |
| "@testing-library/react": "^16.0.0", | |
| "@types/react": "^19.1.9", | |
| "@typescript-eslint/eslint-plugin": "^8.39.0", | |
| "@typescript-eslint/parser": "^8.39.0", | |
| "eslint": "^9.33.0", | |
| "eslint-config-prettier": "^10.1.8", | |
| "eslint-plugin-prettier": "^5.5.4", | |
| "eslint-plugin-react": "^7.37.5", | |
| - "exbuild": "^0.2.2", | |
| + "esbuild": "^0.25.0", | |
| "http-server": "^14.1.1", | |
| "husky": "^9.1.7", | |
| "lint-staged": "^16.1.5", | |
| "prettier": "^3.6.2", | |
| "typescript-eslint": "^8.39.0" | |
| } |
🤖 Prompt for AI Agents
In package.json lines 70 to 87, the devDependency "exbuild" is a typo and should
be replaced with "esbuild" to match the build script usage. Update the
devDependencies by removing "exbuild" and adding "esbuild" with a recent 0.x
version, such as 0.25.8. After updating, run "pnpm install" to update the
lockfile, verify the installation with "pnpm esbuild --version", and then run
"pnpm run build-examples" to confirm the fix.
| try { | ||
| setTimeout(() => { | ||
| if (containerRef.current) { | ||
| const player = new IVLabsPlayer(`#${uniqueIdRef.current}`, playerConfig); | ||
| const player = new IVLabsPlayer(uniqueIdRef.current, playerConfig); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid setTimeout race; initialize synchronously or manage timeout cleanup
Deferring initialization via setTimeout(0) can race with effect cleanup on prop changes, causing double init or init after destroy. Prefer immediate init or track/clear the timeout in cleanup.
Suggested change:
- try {
- setTimeout(() => {
- if (containerRef.current) {
+ try {
+ if (containerRef.current) {
const player = new IVLabsPlayer(uniqueIdRef.current, playerConfig);
playerRef.current = player;
// ...
- }
- }, 0);
+ }
} catch (error) {
console.error('Error initializing IVLabsPlayer:', error);
}If you must defer, store/clear the handle:
- try {
- setTimeout(() => {
+ try {
+ const t = setTimeout(() => {
if (containerRef.current) {
const player = new IVLabsPlayer(uniqueIdRef.current, playerConfig);
playerRef.current = player;
// ...
}
- }, 0);
+ }, 0);
} catch (error) {
console.error('Error initializing IVLabsPlayer:', error);
}
// ...
- return () => {
+ return () => {
+ if (t) clearTimeout(t);
if (playerRef.current) {
playerRef.current.destroy();
playerRef.current = null;
}
};Also applies to: 107-107
🤖 Prompt for AI Agents
In src/index.tsx around lines 68 to 71, avoid using setTimeout to defer player
initialization as it can cause race conditions with effect cleanup, leading to
double initialization or initialization after destruction. Instead, initialize
the player synchronously within the effect. If deferring is necessary, store the
timeout ID in a variable and clear it in the cleanup function to prevent the
callback from running after unmount or prop changes. Apply the same fix around
line 107.
|
@Bmongo Check the reviews and take action. |
|
@Bmongo check the coderabbit review as well |
2 participants
Summary by CodeRabbit
Chores
Style
New Features
Documentation
Tests