v6 #1189
v6 #1189
Conversation
- Updated various dependencies in package.json to use the latest versions. - Refactored Plyr import and usage in src/index.tsx to align with the new module structure. - Removed unused __DEV__ variable from types.d.ts. - Adjusted test file to use @ts-expect-error for better type handling. - Changed module resolution strategy in tsconfig.json from "node" to "bundler".
- Updated build script to use tsdown with a custom config and minification. - Removed rollup configuration file and replaced it with a custom plugin for copying CSS. - Reintroduced plyr as a dependency in package.json. - Adjusted usePlyr function to return a React.Ref type. - Enhanced PlyrComponent type definitions for better TypeScript support. - Cleaned up package-lock.json by removing unnecessary dependencies.
- Added package.json with dependencies for React, Plyr, and Vite. - Included Vite configuration for development and production builds. - Created main application structure with App component and styles. - Integrated Plyr for video playback with a sample YouTube video. - Added SVG assets for branding and icons. - Implemented global styles and responsive design for the application.
|
|
WalkthroughReplaces legacy build and lint tooling, restructures package exports to dist/, switches TS resolution, adds a rolldown build and new Vite example, removes multiple example projects and configs, migrates ESLint to flat config, and changes the component export from default to a named Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Consumer App
participant Lib as plyr-react (src/index.tsx)
participant Plyr as Plyr JS
Dev->>Lib: import { Plyr, usePlyr }
Dev->>Lib: <Plyr ref={ref} source options />
activate Lib
Lib->>Plyr: instantiate(source, options)
Plyr-->>Lib: returns PlyrInstance
Lib-->>Dev: ref.current = PlyrInstance
deactivate Lib
note right of Lib: Export changed: default → named { Plyr }
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
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 |
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Docstrings generation was requested by @chintan9. * #1189 (comment) The following files were modified: * `example/plyr-example/src/App.jsx` * `src/index.tsx`
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Note Generated docstrings for this pull request at #1190 |
📝 Add docstrings to `tsdown`
|
@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll help you analyze the scheduler package and create both a class diagram and README. Let me start by exploring the structure and reading the files. 🏁 Script executed: Length of output: 711 Let me check if there's a scheduler package or directory elsewhere in the codebase. 🏁 Script executed: Length of output: 1079 I can see that there's no 🏁 Script executed: Length of output: 5326 @chintan9 I've thoroughly searched the codebase, and there is no What I FoundThis is the plyr-react repository - a React wrapper for the Plyr media player library. The current structure is: The codebase contains:
Clarification NeededCould you please clarify what you'd like me to do? Here are some possibilities:
Please let me know which option matches your intention, and I'll be happy to help! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
README.md (2)
23-25: Update demo link to new example pathThe link still targets example/react. The new example lives at example/plyr-example.
-You can see a live demo [here](https://githubbox.com/chintan9/plyr-react/tree/master/example/react). +You can see a live demo [here](https://githubbox.com/chintan9/plyr-react/tree/master/example/plyr-example).
262-269: Fix heading level and minor grammarHeading jumps from h2→h4 and lints complain. Also tighten phrasing.
-#### Acknowledgments and Key Contributors +### Acknowledgments and Key Contributors @@ -- **@mnervik:** Provided valuable support through testing and user feedback. +- **@mnervik:** Provided valuable support with testing and user feedback.tests/Plyr.test.tsx (1)
6-11: Fix Jest mock for default ESM export and remove duplicate mock.Current mock won’t populate
Plyr.defaultreliably. Provide__esModuleanddefault.-// https://github.com/jsdom/jsdom/issues/2541#issuecomment-788761237 -jest.mock("plyr"); -jest.mock("plyr", () => { - return jest - .fn() - .mockImplementation(() => ({ destroy: jest.fn(), playing: false })); -}); +// https://github.com/jsdom/jsdom/issues/2541#issuecomment-788761237 +jest.mock("plyr", () => ({ + __esModule: true, + default: jest + .fn() + .mockImplementation(() => ({ destroy: jest.fn(), playing: false })), +}));
🧹 Nitpick comments (22)
tsconfig.json (1)
10-10: Bundler resolution: opt into ESM and preserve import syntax"lib": ["es2018", "dom", "scripthost", "es2015.proxy"], "moduleResolution": "bundler", + "module": "ESNext", + "verbatimModuleSyntax": true,Verification (requires local TypeScript install):
npm install --no-save typescript npx tsc -p tsconfig.json --noEmit.github/workflows/nodejs.yml (1)
21-21: npm@11 upgrade: add cache and consider Actions v4Good to standardize on npm 11. For faster CI and newer Node 24 support, enable dependency cache and bump Actions to v4.
Add caching (outside the edited line):
- uses: actions/setup-node@v4 with: node-version: ${{ matrix.node-version }} cache: npmOptionally bump checkout:
- uses: actions/checkout@v4Please verify Windows runners still succeed after these bumps.
.husky/pre-push (1)
4-4: Pre-push runs full lint/build/test — consider speed-upsLooks fine. To keep pushes snappy, consider lint-staged on pre-commit and running only tests here, or allow an opt-out via
SKIP_PREPUSH=1.Example (if desired):
[ -n "$SKIP_PREPUSH" ] && exit 0 npm run lint && npm run build && npm testsrc/types.d.ts (1)
1-2: Mark src/types.d.ts as a module to avoid global augmentations--- a/src/types.d.ts +++ b/src/types.d.ts @@ 1,2c1,2 -// This file can be empty or you can add other global types. -// For now, it is intentionally left empty as __DEV__ is no longer used. +// Intentionally empty: no global types. +export {};
__DEV__is only declared in package.json and in the removed comment; no other references remain.example/plyr-example/README.md (1)
1-12: Mention Plyr CSS import for correct stylingA brief note helps new users avoid unstyled controls.
## Expanding the ESLint configuration @@ If you are developing a production application, we recommend using TypeScript with type-aware lint rules enabled. Check out the [TS template](https://github.com/vitejs/vite/tree/main/packages/create-vite/template-react-ts) for information on how to integrate TypeScript and [`typescript-eslint`](https://typescript-eslint.io) in your project. + +## Plyr CSS +Remember to include Plyr’s CSS in your app entry: + +```js +import "plyr/dist/plyr.css"; +```example/plyr-example/vite.config.js (1)
5-15: Expose server for Gitpod and lock port for stability.Add
host: trueso the dev server listens on all interfaces in remote/Gitpod, andstrictPort: trueto avoid port fallbacks.export default defineConfig({ plugins: [react()], server: { + host: true, + strictPort: true, // This allows Vite to be accessed from the Gitpod preview URL hmr: { clientPort: 443, }, // Respond to all hosts, which is needed for Gitpod allowedHosts: [".gitpod.io"], }, });example/plyr-example/src/index.css (1)
25-31: Deduplicate body styles to avoid conflicting overrides.Merge the second
bodyblock into the first; keep a single source of truth.body { margin: 0; display: flex; place-items: center; min-width: 320px; min-height: 100vh; + justify-content: center; + padding: 2rem; } @@ -/* src/index.css */ -body { - font-family: system-ui, sans-serif; - background-color: #1a1a1a; - color: white; - display: flex; - justify-content: center; - padding: 2rem; -}Also applies to: 70-78
.lintstagedrc.json (1)
2-3: Broaden patterns to catch common file types.Include mjs/cjs in linting and yml/yaml/mdx in formatting for typical repo churn.
- "*.{ts,tsx,js,jsx}": ["eslint --fix", "prettier --write"], - "*.{json,md,css}": "prettier --write" + "*.{ts,tsx,js,jsx,mjs,cjs}": ["eslint --fix", "prettier --write"], + "*.{json,json5,md,mdx,css,yml,yaml}": "prettier --write"eslint.config.js (2)
12-13: Expand coverage and ignores.
- Ignore nested dist folders across packages/examples.
- Lint JS/JSX too (example app uses .jsx).
- Broaden test globs to match common patterns.
- ignores: ["dist/", "node_modules/", "coverage/"], + ignores: ["**/dist/**", "node_modules/", "coverage/"], @@ - files: ["**/*.{ts,tsx}"], + files: ["**/*.{ts,tsx,js,jsx}"], @@ - files: ["tests/**/*.{ts,tsx}"], + files: [ + "tests/**/*.{ts,tsx,js,jsx}", + "**/__tests__/**/*.{ts,tsx,js,jsx}", + "**/*.{test,spec}.{ts,tsx,js,jsx}" + ],Also applies to: 20-21, 45-46
36-40: Tighten import resolver for TS path/types.If you use TS path aliases or type-only packages, configure the resolver accordingly.
"import/resolver": { - typescript: true, - node: true, + typescript: { alwaysTryTypes: true, projectService: true }, + node: true },example/plyr-example/eslint.config.js (2)
16-24: Avoid conflicting ECMAScript versions in flat configYou set both languageOptions.ecmaVersion (2020) and parserOptions.ecmaVersion ("latest"). Pick one to avoid ambiguity; in ESLint v9 flat configs, prefer languageOptions.ecmaVersion and drop the duplicate in parserOptions.
languageOptions: { - ecmaVersion: 2020, + ecmaVersion: "latest", globals: globals.browser, - parserOptions: { - ecmaVersion: "latest", - ecmaFeatures: { jsx: true }, - sourceType: "module", - }, + parserOptions: { ecmaFeatures: { jsx: true }, sourceType: "module" }, },
10-15: Optional: Limit linting to source files to speed up CIIf the example grows, consider scoping files to src/**/* to avoid linting config/build outputs.
- files: ["**/*.{js,jsx}"], + files: ["src/**/*.{js,jsx}"],README.md (1)
170-170: Tiny wording polish“gives you access” → “provides access”.
-The ref gives you access to the full Plyr API. +The ref provides access to the full Plyr API.example/plyr-example/package.json (2)
12-17: Use local workspace dependency for iterative devPoint the example to the local package to ensure it tests the current PR build. Keep "next" only for external consumers.
"dependencies": { "plyr": "^3.8.3", - "plyr-react": "next", + "plyr-react": "workspace:*", "react": "^19.1.1", "react-dom": "^19.1.1" },
1-11: Pin Node engine to avoid dev surprisesGiven Vite 7 and React 19, declare an engines.node to >=18.
{ "name": "plyr-example", "private": true, "version": "0.0.0", "type": "module", + "engines": { "node": ">=18" }, "scripts": {package.json (1)
97-109: Refine Jest config: narrow test matching and exclude tests from coverage.The current regex is broad and coverage includes tests.
- "testRegex": "test.(ts|tsx)$", + "testRegex": "(/__tests__/.*|(\\.|/)(test|spec))\\.(ts|tsx)$", - "collectCoverageFrom": [ - "src/**/*.{ts,tsx}", - "tests/**/*.{ts,tsx}" - ], + "collectCoverageFrom": ["src/**/*.{ts,tsx}"],src/index.tsx (1)
74-79: Tighten return type ofusePlyrto avoid casts.Return
MutableRefObject<HTMLVideoElement>directly.export function usePlyr( ref: Ref<APITypes>, params: PlyrConfigurationProps, deps: DependencyList | null = null -): React.Ref<HTMLVideoElement> { +): MutableRefObject<HTMLVideoElement> { return useAptor<PlyrInstance, HTMLVideoElement, PlyrConfigurationProps>(- const raptorRef = usePlyr(ref, { + const raptorRef = usePlyr(ref, { source, options, - }) as MutableRefObject<HTMLVideoElement>; + });Also applies to: 95-99
example/plyr-example/index.html (1)
7-7: Use a more descriptive page title.Recommend naming it after the example for clarity.
- <title>Vite + React</title> + <title>plyr-react v6 — Example</title>example/plyr-example/src/main.jsx (2)
3-6: Prefer library CSS before app CSS to simplify overrides.Import Plyr CSS first so your styles can override it more predictably.
-import "./index.css"; +import "plyr-react/plyr.css"; +import "./index.css"; -import "plyr-react/plyr.css";
8-12: Guard against missing root element.Avoid a runtime TypeError if
#rootis absent.-createRoot(document.getElementById("root")).render( +const rootEl = document.getElementById("root"); +if (!rootEl) throw new Error("Root element #root not found"); +createRoot(rootEl).render( <StrictMode> <App /> </StrictMode> );If StrictMode double-mount reveals any side effects with Plyr initialization in dev, consider confirming cleanup behavior.
example/plyr-example/src/App.jsx (2)
21-24: Make the heading environment-agnostic.“GitHub Codespaces” may not apply for all users.
- <h1>Plyr React in GitHub Codespaces</h1> + <h1>Plyr React Example</h1>
1-1: Optional: Import component styles in App.jsx
src/App.cssexists but isn’t imported insrc/App.jsx. To apply those styles, add:// src/App.jsx + import "./App.css";Or, if you don’t need the stylesheet, remove the
Appclass (or add minimal inline styles).
📜 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 (25)
dist/LICENSEis excluded by!**/dist/**dist/README.mdis excluded by!**/dist/**dist/esm/index.d.tsis excluded by!**/dist/**dist/esm/index.jsis excluded by!**/dist/**dist/esm/index.mjsis excluded by!**/dist/**dist/index.cjsis excluded by!**/dist/**dist/index.cjs.mapis excluded by!**/dist/**,!**/*.mapdist/index.d.ctsis excluded by!**/dist/**dist/index.d.tsis excluded by!**/dist/**dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.mapdist/package.jsonis excluded by!**/dist/**dist/plyr.cssis excluded by!**/dist/**dist/system/index.development.jsis excluded by!**/dist/**dist/system/index.production.jsis excluded by!**/dist/**dist/umd/index.development.jsis excluded by!**/dist/**dist/umd/index.production.jsis excluded by!**/dist/**example/nextjs/public/favicon.icois excluded by!**/*.icoexample/nextjs/public/vercel.svgis excluded by!**/*.svgexample/nextjs/yarn.lockis excluded by!**/yarn.lock,!**/*.lockexample/plyr-example/package-lock.jsonis excluded by!**/package-lock.jsonexample/plyr-example/public/vite.svgis excluded by!**/*.svgexample/plyr-example/src/assets/react.svgis excluded by!**/*.svgexample/react/yarn.lockis excluded by!**/yarn.lock,!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (48)
.eslintignore(0 hunks).eslintrc.json(0 hunks).github/workflows/nodejs.yml(1 hunks).husky/pre-push(1 hunks).lintstagedrc.json(1 hunks)README.md(3 hunks)babel.config.js(0 hunks)eslint.config.js(1 hunks)example/nextjs/.eslintrc.json(0 hunks)example/nextjs/.gitignore(0 hunks)example/nextjs/README.md(0 hunks)example/nextjs/next.config.js(0 hunks)example/nextjs/package.json(0 hunks)example/nextjs/pages/_app.js(0 hunks)example/nextjs/pages/api/hello.js(0 hunks)example/nextjs/pages/index.js(0 hunks)example/nextjs/src/contents.json(0 hunks)example/nextjs/src/video-list.jsx(0 hunks)example/nextjs/styles/globals.css(0 hunks)example/plyr-example/.gitignore(1 hunks)example/plyr-example/README.md(1 hunks)example/plyr-example/eslint.config.js(1 hunks)example/plyr-example/index.html(1 hunks)example/plyr-example/package.json(1 hunks)example/plyr-example/src/App.css(1 hunks)example/plyr-example/src/App.jsx(1 hunks)example/plyr-example/src/index.css(1 hunks)example/plyr-example/src/main.jsx(1 hunks)example/plyr-example/vite.config.js(1 hunks)example/react/README.md(0 hunks)example/react/package.json(0 hunks)example/react/src/audio-player/custom-audio-player.tsx(0 hunks)example/react/src/hls-player/custom-hls-player.tsx(0 hunks)example/react/src/index.css(0 hunks)example/react/src/main.tsx(0 hunks)example/react/src/video-list/contents.json(0 hunks)example/react/src/video-list/video-list.tsx(0 hunks)example/react/src/vite-env.d.ts(0 hunks)example/react/tsconfig.json(0 hunks)example/react/vite.config.ts(0 hunks)jest.config.ts(0 hunks)package.json(1 hunks)rolldown.config.ts(1 hunks)rollup.config.js(0 hunks)src/index.tsx(5 hunks)src/types.d.ts(1 hunks)tests/Plyr.test.tsx(2 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (27)
- example/react/README.md
- example/react/src/video-list/video-list.tsx
- example/react/src/video-list/contents.json
- example/react/src/index.css
- example/nextjs/.gitignore
- example/nextjs/src/video-list.jsx
- example/react/src/main.tsx
- .eslintrc.json
- example/nextjs/src/contents.json
- babel.config.js
- rollup.config.js
- .eslintignore
- example/react/vite.config.ts
- example/react/src/hls-player/custom-hls-player.tsx
- example/nextjs/package.json
- example/react/package.json
- example/nextjs/styles/globals.css
- example/nextjs/pages/api/hello.js
- example/nextjs/README.md
- example/nextjs/pages/_app.js
- example/react/tsconfig.json
- example/nextjs/pages/index.js
- example/react/src/vite-env.d.ts
- jest.config.ts
- example/nextjs/next.config.js
- example/nextjs/.eslintrc.json
- example/react/src/audio-player/custom-audio-player.tsx
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~170-~170: Try using a synonym here to strengthen your writing.
Context: ...rogrammatically by using a ref. The ref gives you access to the full Plyr API. The ref's ...
(GIVE_PROVIDE)
[grammar] ~266-~266: There might be a mistake here.
Context: ...ding the integration with react-aptor. - @iwatakeshi (Takeshi): Led the convers...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
README.md
60-60: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
262-262: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
🔇 Additional comments (14)
example/plyr-example/vite.config.js (1)
1-6: ESM config validated
The example’s package.json already has"type": "module", sovite.config.jscan useimportwithout renaming or additional flags.example/plyr-example/.gitignore (1)
1-24: LGTM — solid baseline ignores.rolldown.config.ts (2)
31-33: package.json exports './plyr.css' to './dist/plyr.css' as expected.
29-36: No action needed:rolldown.config.tsis explicitly loaded via--config rolldown.config.tsin your build and dev scripts.README.md (3)
142-144: Ensure CSS path matches published artifactsDocs say import "plyr-react/plyr.css". Verify package.json exports exposes ./plyr.css (mapped to dist/plyr.css) or adjust docs back to "plyr-react/dist/plyr.css".
101-104: Verify Plyr export style in docs
Grep on local sources didn’t locate anyexport default Plyrorexport { Plyr }insrc/index.tsx, so please manually confirm whetherplyr-reactv6 exportsPlyras a default or named export and update the imports in README.md (lines 101–104, 137–139) accordingly.
88-92: Raise Node.js minimum to match toolchainVite 7 and React 19 generally require Node 18+. Update docs to prevent setup issues.
-- **Node.js:** Version `16` or higher. +- **Node.js:** Version `18` or higher.Likely an incorrect or invalid review comment.
example/plyr-example/package.json (1)
12-17: Optional: align plyr version with READMEREADME suggests upgrading plyr peer to ^3.8.3; example already uses it—good. Ensure the root package.json peerDependencies match.
example/plyr-example/src/App.css (1)
1-43: LGTMStyling is standard Vite starter; no functional concerns.
package.json (1)
20-31: Module packaging/exports look correct.ESM + CJS dual entry via exports map is well-structured; TS
typescondition present. Nice.example/plyr-example/index.html (2)
11-11: LGTM on module entry.Standard Vite pattern; no issues.
5-5: Resolved: favicon path is valid
vite.svgexists inexample/plyr-example/public, so referencing it as/vite.svgwill serve without a 404.example/plyr-example/src/main.jsx (1)
1-2: React 18 entrypoint looks correct.example/plyr-example/src/App.jsx (1)
4-4: Import style matches v6 named export.
{ Plyr }aligns with the new public API. Good.
| }, | ||
| "license": "MIT", | ||
| "author": "Chintan Prajapati", | ||
| "sideEffects": false, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix tree-shaking of CSS: mark CSS as having side effects.
With "sideEffects": false, bundlers can drop import "plyr-react/plyr.css". Declare CSS as side-effectful to prevent accidental removal.
Apply:
- "sideEffects": false,
+ "sideEffects": ["*.css", "dist/plyr.css"],Also applies to: 26-26
🤖 Prompt for AI Agents
In package.json around lines 19 and 26, "sideEffects": false prevents CSS
imports (e.g., "plyr-react/plyr.css") from being preserved during tree-shaking;
replace the boolean with an explicit list of side-effectful file patterns such
as ["**/*.css","*.css"] or include the specific CSS paths you import (or set to
true) so CSS files are not removed by bundlers.
| "build": "tsdown --config rolldown.config.ts --minify", | ||
| "dev": "tsdown --config rolldown.config.ts --watch", | ||
| "lint": "eslint --fix '{src,tests}/**/*.{ts,tsx}'", | ||
| "test": "jest", | ||
| "test:ci": "jest", | ||
| "test:coverage": "jest --coverage --forceExit --colors", | ||
| "test:coverage:watch": "jest --watch", | ||
| "test:dev": "jest --watch --no-coverage", | ||
| "lint": "esw --ext .tsx --ext .ts --color", | ||
| "lint:fix": "npm run lint --fix", | ||
| "eslint": "eslint --fix '*.{js,json}' '{src,tests}/**/*.{ts,tsx}'", | ||
| "eslint:ci": "eslint '*.{js,json}' '{src,tests}/**/*.{ts,tsx}'", | ||
| "prebuild": "shx rm -rf dist", | ||
| "build": "rollup -c", | ||
| "postbuild": "npm run copy", | ||
| "precopy": "shx cp -r dist/src/* dist/esm && shx cp -r dist/src/* dist && shx rm -rf dist/src && shx rm -rf dist/{src,tests}", | ||
| "copy": "concurrently -m 8 'npm:copy:*'", | ||
| "copy:package-json": "shx cp package.json dist && json -I -f dist/package.json -e 'this.private=false; this.devDependencies=undefined; this.scripts=undefined; this.publishConfig=undefined'", | ||
| "copy:static-content": "shx cp README.md LICENSE dist", | ||
| "prepare": "npm run build && husky install", | ||
| "prerelease": "npm run build", | ||
| "release": "npm publish", | ||
| "predeploy": "npm run prepare", | ||
| "deploy": "gh-pages -d" | ||
| "prepare": "husky install", | ||
| "release": "npm publish" | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure builds are published reliably.
Add a guard to prevent publishing without a fresh dist.
"scripts": {
"build": "tsdown --config rolldown.config.ts --minify",
"dev": "tsdown --config rolldown.config.ts --watch",
"lint": "eslint --fix '{src,tests}/**/*.{ts,tsx}'",
"test": "jest",
"prepare": "husky install",
- "release": "npm publish"
+ "release": "npm publish",
+ "prepublishOnly": "npm run build"
},📝 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.
| "build": "tsdown --config rolldown.config.ts --minify", | |
| "dev": "tsdown --config rolldown.config.ts --watch", | |
| "lint": "eslint --fix '{src,tests}/**/*.{ts,tsx}'", | |
| "test": "jest", | |
| "test:ci": "jest", | |
| "test:coverage": "jest --coverage --forceExit --colors", | |
| "test:coverage:watch": "jest --watch", | |
| "test:dev": "jest --watch --no-coverage", | |
| "lint": "esw --ext .tsx --ext .ts --color", | |
| "lint:fix": "npm run lint --fix", | |
| "eslint": "eslint --fix '*.{js,json}' '{src,tests}/**/*.{ts,tsx}'", | |
| "eslint:ci": "eslint '*.{js,json}' '{src,tests}/**/*.{ts,tsx}'", | |
| "prebuild": "shx rm -rf dist", | |
| "build": "rollup -c", | |
| "postbuild": "npm run copy", | |
| "precopy": "shx cp -r dist/src/* dist/esm && shx cp -r dist/src/* dist && shx rm -rf dist/src && shx rm -rf dist/{src,tests}", | |
| "copy": "concurrently -m 8 'npm:copy:*'", | |
| "copy:package-json": "shx cp package.json dist && json -I -f dist/package.json -e 'this.private=false; this.devDependencies=undefined; this.scripts=undefined; this.publishConfig=undefined'", | |
| "copy:static-content": "shx cp README.md LICENSE dist", | |
| "prepare": "npm run build && husky install", | |
| "prerelease": "npm run build", | |
| "release": "npm publish", | |
| "predeploy": "npm run prepare", | |
| "deploy": "gh-pages -d" | |
| "prepare": "husky install", | |
| "release": "npm publish" | |
| }, | |
| "scripts": { | |
| "build": "tsdown --config rolldown.config.ts --minify", | |
| "dev": "tsdown --config rolldown.config.ts --watch", | |
| "lint": "eslint --fix '{src,tests}/**/*.{ts,tsx}'", | |
| "test": "jest", | |
| "prepare": "husky install", | |
| "release": "npm publish", | |
| "prepublishOnly": "npm run build" | |
| }, |
🤖 Prompt for AI Agents
In package.json around lines 37–43, the "release" script can publish stale or
missing builds; change it to guard by ensuring a fresh dist before publishing —
either add a prepublishOnly script that runs "npm run build" (and optionally
verifies the dist directory exists and is non-empty) or replace "release" with a
compound command that runs npm run build && npm publish; ensure the build step
fails the publish if dist is missing or the build exits non-zero.
| "@typescript-eslint/eslint-plugin": "latest", | ||
| "@typescript-eslint/parser": "latest", | ||
| "eslint": "latest", | ||
| "eslint-config-prettier": "latest", | ||
| "eslint-import-resolver-typescript": "^4.4.4", | ||
| "eslint-plugin-import": "latest", | ||
| "eslint-plugin-jest": "latest", | ||
| "eslint-plugin-prettier": "latest", | ||
| "eslint-plugin-react": "latest", | ||
| "eslint-plugin-react-hooks": "latest", | ||
| "husky": "^8.0.3", | ||
| "jasmine-expect": "^5.0.0", | ||
| "jest": "^27.5.1", | ||
| "jest-environment-jsdom": "^27.5.1", | ||
| "jest-pnp-resolver": "^1.2.3", | ||
| "jest-resolve": "^28.1.3", | ||
| "jest-watch-typeahead": "^2.2.2", | ||
| "json": "^11.0.0", | ||
| "lint-staged": "^12.5.0", | ||
| "npm-run-all": "^4.1.5", | ||
| "prettier": "^2.8.6", | ||
| "prop-types": "^15.8.1", | ||
| "plyr": "^3.7.7", | ||
| "prettier": "latest", | ||
| "react": "^17.0.2", | ||
| "react-app-polyfill": "^3.0.0", | ||
| "react-docgen-typescript": "^2.2.2", | ||
| "react-dom": "^17.0.2", | ||
| "react-error-overlay": "^6.0.11", | ||
| "react-test-renderer": "^17.0.2", | ||
| "rollup": "^2.79.1", | ||
| "rollup-plugin-copy": "^3.4.0", | ||
| "rollup-plugin-esbuild": "^4.10.3", | ||
| "rollup-plugin-peer-deps-external": "^2.2.4", | ||
| "rollup-plugin-terser": "^7.0.2", | ||
| "sass": "^1.59.3", | ||
| "sass-loader": "^12.6.0", | ||
| "shx": "^0.3.4", | ||
| "sort-package-json": "^1.57.0", | ||
| "style-loader": "^3.3.2", | ||
| "ts-node": "^10.9.1", | ||
| "tsdown": "^0.2.0", | ||
| "typescript": "^5.0.4", | ||
| "webpack": "^5.76.2" | ||
| "typescript-eslint": "^8.41.0" | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid “latest” churn; align engines with tooling.
Multiple ESLint/TS-ESLint deps are set to "latest", which can break builds unexpectedly and often require Node ≥18. Recommend pinning compatible versions and/or bumping engines to Node 18+.
Would you confirm the required Node versions for your chosen ESLint/TypeScript-ESLint stack and whether you want to bump engines?
If bumping engines:
"engines": {
- "node": ">=16"
+ "node": ">=18.18"
},Also applies to: 81-82
🌐 Web query:
What Node versions are required by ESLint v9 and typescript-eslint v8? Are they compatible with Node 16?
💡 Result:
Short answer: No — both require Node ≥18.18.0 (or the newer 20/21 ranges) and do not support Node 16.
Details:
- ESLint v9 requires Node.js ^18.18.0 || ^20.9.0 || >=21.1.0 (ESLint v9 dropped Node <18.18 and v19). [1]
- @typescript-eslint v8 uses the same Node engine range (^18.18.0 || ^20.9.0 || >=21.1.0). [2]
If you must stay on Node 16, remain on ESLint v8 (and a typescript-eslint release that still supports Node 16). [1][2]
Sources:
[1] ESLint v9 release/migration notes.
[2] typescript-eslint dependency versions / engine requirements.
Pin ESLint/TS-ESLint deps and bump Node engine to ≥18.18.0
Pin all ESLint-related packages to known versions (e.g. eslint@9.x, @typescript-eslint@8.x, eslint-plugin-import@2.x, etc.) instead of "latest" to avoid unexpected breakages (ESLint v9 and @typescript-eslint v8 both require Node ≥18.18.0). Then update your package.json:
"engines": {
- "node": ">=16"
+ "node": ">=18.18.0"
},Also apply this change on lines 81–82.
📝 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.
| "@typescript-eslint/eslint-plugin": "latest", | |
| "@typescript-eslint/parser": "latest", | |
| "eslint": "latest", | |
| "eslint-config-prettier": "latest", | |
| "eslint-import-resolver-typescript": "^4.4.4", | |
| "eslint-plugin-import": "latest", | |
| "eslint-plugin-jest": "latest", | |
| "eslint-plugin-prettier": "latest", | |
| "eslint-plugin-react": "latest", | |
| "eslint-plugin-react-hooks": "latest", | |
| "husky": "^8.0.3", | |
| "jasmine-expect": "^5.0.0", | |
| "jest": "^27.5.1", | |
| "jest-environment-jsdom": "^27.5.1", | |
| "jest-pnp-resolver": "^1.2.3", | |
| "jest-resolve": "^28.1.3", | |
| "jest-watch-typeahead": "^2.2.2", | |
| "json": "^11.0.0", | |
| "lint-staged": "^12.5.0", | |
| "npm-run-all": "^4.1.5", | |
| "prettier": "^2.8.6", | |
| "prop-types": "^15.8.1", | |
| "plyr": "^3.7.7", | |
| "prettier": "latest", | |
| "react": "^17.0.2", | |
| "react-app-polyfill": "^3.0.0", | |
| "react-docgen-typescript": "^2.2.2", | |
| "react-dom": "^17.0.2", | |
| "react-error-overlay": "^6.0.11", | |
| "react-test-renderer": "^17.0.2", | |
| "rollup": "^2.79.1", | |
| "rollup-plugin-copy": "^3.4.0", | |
| "rollup-plugin-esbuild": "^4.10.3", | |
| "rollup-plugin-peer-deps-external": "^2.2.4", | |
| "rollup-plugin-terser": "^7.0.2", | |
| "sass": "^1.59.3", | |
| "sass-loader": "^12.6.0", | |
| "shx": "^0.3.4", | |
| "sort-package-json": "^1.57.0", | |
| "style-loader": "^3.3.2", | |
| "ts-node": "^10.9.1", | |
| "tsdown": "^0.2.0", | |
| "typescript": "^5.0.4", | |
| "webpack": "^5.76.2" | |
| "typescript-eslint": "^8.41.0" | |
| }, | |
| "engines": { | |
| "node": ">=18.18.0" | |
| }, |
🤖 Prompt for AI Agents
In package.json around lines 54 to 75 (and also update lines 81–82 as noted),
replace all eslint-related dependencies currently set to "latest" with explicit,
compatible version ranges (for example: "eslint": "9.x",
"@typescript-eslint/eslint-plugin": "8.x", "@typescript-eslint/parser": "8.x",
"eslint-plugin-import": "2.x", "eslint-plugin-react": "7.x",
"eslint-plugin-react-hooks": "4.x", "eslint-plugin-prettier": "4.x",
"eslint-config-prettier": "9.x", etc.) and remove any duplicate/incorrect keys
like "typescript-eslint": keep only @typescript-eslint packages; then update the
package.json "engines" field (or add one) to require Node >=18.18.0 (e.g.
"node": ">=18.18.0") so the pinned ESLint/TS-ESLint versions are supported;
ensure lines 81–82 reflect the same pinned versions if they list related deps or
scripts.
| For `plyr-react` to function correctly, it requires you to install some packages in your project yourself. These are known as peer dependencies. The `package.json` file specifies the following: | ||
|
|
||
| - **`react`**: Version `16.8` or newer. | ||
| - **`plyr`**: Version `3.7.7` or a compatible version. | ||
|
|
||
| You must ensure these are listed in your project's `package.json`. Most package managers will warn you if these are missing. |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align peer dependency versions with example and build
Peer deps list plyr 3.7.7 while the example uses ^3.8.3. Recommend aligning to the same minimum.
-- **`plyr`**: Version `3.7.7` or a compatible version.
+- **`plyr`**: Version `^3.8.3` or newer compatible version.📝 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.
| For `plyr-react` to function correctly, it requires you to install some packages in your project yourself. These are known as peer dependencies. The `package.json` file specifies the following: | |
| - **`react`**: Version `16.8` or newer. | |
| - **`plyr`**: Version `3.7.7` or a compatible version. | |
| You must ensure these are listed in your project's `package.json`. Most package managers will warn you if these are missing. | |
| For `plyr-react` to function correctly, it requires you to install some packages in your project yourself. These are known as peer dependencies. The `package.json` file specifies the following: | |
| - **`react`**: Version `16.8` or newer. | |
| - **`plyr`**: Version `^3.8.3` or newer compatible version. | |
| You must ensure these are listed in your project's `package.json`. Most package managers will warn you if these are missing. |
🤖 Prompt for AI Agents
In README.md around lines 66 to 71, the peer dependency version for plyr
(currently listed as 3.7.7) is inconsistent with the example which uses ^3.8.3;
update the README to specify the same minimum version (e.g., ^3.8.3) so
documentation matches the example/build, and while here verify and, if relevant,
update the package.json peerDependencies to the same version to keep docs and
package metadata aligned.
| const source = path.resolve(__dirname, "node_modules/plyr/dist/plyr.css"); | ||
| const destinationDir = path.resolve(__dirname, "dist"); | ||
| const destinationFile = path.join(destinationDir, "plyr.css"); | ||
|
|
||
| // Ensure the 'dist' directory exists. The recursive option prevents errors if it already exists. | ||
| fs.mkdirSync(destinationDir, { recursive: true }); | ||
|
|
||
| // Copy the file | ||
| fs.copyFileSync(source, destinationFile); | ||
|
|
||
| console.log("✅ Copied plyr.css to dist/plyr.css"); | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make CSS path resolution robust and fail-safe
Hardcoding node_modules/plyr/... is brittle in workspaces/PNPM. Resolve via Node’s resolver and handle missing peer gracefully.
-import fs from "fs";
+import fs from "fs";
import path from "path";
import { fileURLToPath } from "url";
+import { createRequire } from "module";
const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);
+const require = createRequire(import.meta.url);
@@
// This function runs after the bundle is written to the dist folder
- writeBundle() {
- const source = path.resolve(__dirname, "node_modules/plyr/dist/plyr.css");
- const destinationDir = path.resolve(__dirname, "dist");
- const destinationFile = path.join(destinationDir, "plyr.css");
-
- // Ensure the 'dist' directory exists. The recursive option prevents errors if it already exists.
- fs.mkdirSync(destinationDir, { recursive: true });
-
- // Copy the file
- fs.copyFileSync(source, destinationFile);
-
- console.log("✅ Copied plyr.css to dist/plyr.css");
- },
+ writeBundle() {
+ const destinationDir = path.resolve(__dirname, "dist");
+ const destinationFile = path.join(destinationDir, "plyr.css");
+ fs.mkdirSync(destinationDir, { recursive: true });
+ try {
+ const source = require.resolve("plyr/dist/plyr.css");
+ fs.copyFileSync(source, destinationFile);
+ console.log("✅ Copied plyr.css to dist/plyr.css");
+ } catch (err) {
+ console.warn("⚠️ plyr/dist/plyr.css not found. Is 'plyr' installed as a peer dependency?");
+ }
+ },📝 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.
| const source = path.resolve(__dirname, "node_modules/plyr/dist/plyr.css"); | |
| const destinationDir = path.resolve(__dirname, "dist"); | |
| const destinationFile = path.join(destinationDir, "plyr.css"); | |
| // Ensure the 'dist' directory exists. The recursive option prevents errors if it already exists. | |
| fs.mkdirSync(destinationDir, { recursive: true }); | |
| // Copy the file | |
| fs.copyFileSync(source, destinationFile); | |
| console.log("✅ Copied plyr.css to dist/plyr.css"); | |
| }, | |
| import fs from "fs"; | |
| import path from "path"; | |
| import { fileURLToPath } from "url"; | |
| import { createRequire } from "module"; | |
| const __filename = fileURLToPath(import.meta.url); | |
| const __dirname = path.dirname(__filename); | |
| const require = createRequire(import.meta.url); | |
| export default { | |
| // ... other Rollup config ... | |
| // This function runs after the bundle is written to the dist folder | |
| writeBundle() { | |
| const destinationDir = path.resolve(__dirname, "dist"); | |
| const destinationFile = path.join(destinationDir, "plyr.css"); | |
| fs.mkdirSync(destinationDir, { recursive: true }); | |
| try { | |
| const source = require.resolve("plyr/dist/plyr.css"); | |
| fs.copyFileSync(source, destinationFile); | |
| console.log("✅ Copied plyr.css to dist/plyr.css"); | |
| } catch (err) { | |
| console.warn( | |
| "⚠️ plyr/dist/plyr.css not found. Is 'plyr' installed as a peer dependency?" | |
| ); | |
| } | |
| }, | |
| }; |
🤖 Prompt for AI Agents
In rolldown.config.ts around lines 15 to 26, the code hardcodes
node_modules/plyr/dist/plyr.css which breaks in workspaces/PNPM; replace the
hardcoded path with Node’s resolver and add graceful handling: call
require.resolve('plyr/dist/plyr.css') (using try/catch and optional paths if
needed) to get the actual file path, verify the resolved path exists before
copying, and if resolution fails or file is missing log a clear warning and skip
the copy (do not throw), ensuring the dist directory is still created when
appropriate.
| import * as Plyr from "plyr"; | ||
| import useAptor, { Destroy, GetAPI, Instantiate } from "react-aptor"; | ||
|
|
||
| export type PlyrInstance = PlyrJS; | ||
| export type PlyrOptions = Options; | ||
| export type PlyrSource = SourceInfo; | ||
| // And this is the critical change for usage | ||
| const PlyrJS = Plyr.default; | ||
|
|
||
| export type PlyrInstance = Plyr.default; | ||
| export type PlyrOptions = Plyr.Options; | ||
| export type PlyrSource = Plyr.SourceInfo; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ESM interop: prefer default import over * as + .default.
This avoids fragile runtime/type paths and simplifies Jest mocking.
-// This is the critical change for the import
-import * as Plyr from "plyr";
-// And this is the critical change for usage
-const PlyrJS = Plyr.default;
-
-export type PlyrInstance = Plyr.default;
-export type PlyrOptions = Plyr.Options;
-export type PlyrSource = Plyr.SourceInfo;
+import PlyrJS from "plyr";
+export type { Options as PlyrOptions, SourceInfo as PlyrSource } from "plyr";
+export type PlyrInstance = PlyrJS;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/index.tsx around lines 11 to 19, the code imports Plyr using a namespace
import and then uses .default which is fragile for ESM interop and Jest mocking;
replace the namespace import with a default (and named types) import from "plyr"
and remove the intermediate Plyr.default assignment. Specifically, change the
import to a default import plus any named type imports (e.g., import Plyr, {
Options, SourceInfo } from "plyr"), then update the exported types to use the
imported identifiers (export type PlyrInstance = Plyr; export type PlyrOptions =
Options; export type PlyrSource = SourceInfo;), and remove the line assigning
PlyrJS = Plyr.default.
| const instantiate: Instantiate< | ||
| PlyrJS, | ||
| PlyrInstance, | ||
| HTMLVideoElement, | ||
| PlyrConfigurationProps | ||
| > = (_, params) => { | ||
| // The node update of ref in react life cycle seems to have issue, used class selector instead | ||
| const plyr = new PlyrJS(".plyr-react", params?.options ?? {}); | ||
| if (params?.source) plyr.source = params?.source; | ||
| return plyr; | ||
| }; |
There was a problem hiding this comment.
Bug: Instantiating Plyr by selector binds the wrong element. Use the provided node.
Using new PlyrJS(".plyr-react", ...) will target the first matching element globally and breaks with multiple instances. Use the actual element passed by react-aptor.
-const instantiate: Instantiate<
- PlyrInstance,
- HTMLVideoElement,
- PlyrConfigurationProps
-> = (_, params) => {
- const plyr = new PlyrJS(".plyr-react", params?.options ?? {});
+const instantiate: Instantiate<
+ PlyrInstance,
+ HTMLVideoElement,
+ PlyrConfigurationProps
+> = (el, params) => {
+ const plyr = new PlyrJS(el, params?.options ?? {});
if (params?.source) plyr.source = params?.source;
return plyr;
};📝 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.
| const instantiate: Instantiate< | |
| PlyrJS, | |
| PlyrInstance, | |
| HTMLVideoElement, | |
| PlyrConfigurationProps | |
| > = (_, params) => { | |
| // The node update of ref in react life cycle seems to have issue, used class selector instead | |
| const plyr = new PlyrJS(".plyr-react", params?.options ?? {}); | |
| if (params?.source) plyr.source = params?.source; | |
| return plyr; | |
| }; | |
| const instantiate: Instantiate< | |
| PlyrInstance, | |
| HTMLVideoElement, | |
| PlyrConfigurationProps | |
| > = (el, params) => { | |
| const plyr = new PlyrJS(el, params?.options ?? {}); | |
| if (params?.source) plyr.source = params?.source; | |
| return plyr; | |
| }; |
🤖 Prompt for AI Agents
In src/index.tsx around lines 36 to 44, the instantiation uses a global selector
new PlyrJS(".plyr-react", ...) which binds the first matching element and breaks
multiple instances; replace the selector with the actual DOM node passed as the
first argument to instantiate (the unused "_" parameter) by casting/typing it to
the expected HTMLVideoElement and calling new PlyrJS(node, params?.options ??
{}), guard that node is non-null before constructing, then set plyr.source if
params?.source and return the instance.
|
Creating a local commit for the generated unit tests... The unit tests have been committed to the current branch. Commit ID: |
|
@coderabbitai revert your unit test changes |
|
Tip For best results, initiate chat on the files or code changes. @chintan9 I understand you'd like to revert the unit test changes, but I cannot directly make commits or revert changes in the repository, even when asked to do so. However, I can help you revert the changes yourself. Here are the options: Option 1: Revert the specific commitgit revert ed062ee545f48d1ca950cba70a1da8c7df1919d1Option 2: Reset to the commit before the unit tests (if it's the latest commit)git reset --hard HEAD~1Option 3: Reset to a specific commit before the unit tests# First, check the git log to see the commit before the unit tests
git log --oneline -5
# Then reset to the commit hash before the unit test commit
git reset --hard <previous-commit-hash>Would you like me to help you identify the exact commands to use, or do you have any questions about reverting the changes? |
|
@coderabbitai Create mermaid digram whole repo |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll analyze the entire repository structure and create a comprehensive Mermaid diagram for you. Now let me read the example files and any build configuration: @chintan9 Here's a comprehensive Mermaid diagram representing the entire repository structure and relationships: graph TB
subgraph "plyr-react Repository"
subgraph "Root Configuration"
PKG["package.json\n📦 Main config"]
TS["tsconfig.json\n🔧 TypeScript config"]
README["README.md\n📖 Documentation"]
GIT[".gitignore\n🚫 Git ignore rules"]
end
subgraph "Source Code (src/)"
INDEX["index.tsx\n🎯 Main component & hooks"]
TYPES["types.d.ts\n📝 Type definitions"]
end
subgraph "Example Application"
EX_PKG["example/package.json\n📦 Example config"]
EX_HTML["example/index.html\n🌐 HTML entry"]
EX_MAIN["example/src/main.tsx\n⚡ Vite app entry"]
EX_APP["example/src/App.tsx\n🎬 Demo application"]
EX_CSS["example/src/index.css\n🎨 Styles"]
end
subgraph "Build Output (dist/)"
DIST_ESM["dist/index.esm.js\n📦 ES Module build"]
DIST_CJS["dist/index.js\n📦 CommonJS build"]
DIST_CSS["dist/plyr.css\n🎨 Plyr styles"]
DIST_TYPES["dist/index.d.ts\n📝 Type declarations"]
end
subgraph "External Dependencies"
PLYR["plyr\n🎥 Media player core"]
REACT["react\n⚛️ React library"]
REACT_DOM["react-dom\n🌐 React DOM"]
end
end
%% Component Relationships
INDEX --|exports|--> PLYR_COMP["Plyr Component"]
INDEX --|exports|--> USE_PLYR["usePlyr Hook"]
INDEX --|exports|--> PLYR_CLASS["Plyr Class"]
%% Type Dependencies
TYPES --|provides types for|--> INDEX
INDEX --|uses|--> PLYR
INDEX --|uses|--> REACT
%% Build Process
INDEX --|builds to|--> DIST_ESM
INDEX --|builds to|--> DIST_CJS
TYPES --|builds to|--> DIST_TYPES
%% Example Dependencies
EX_MAIN --|imports|--> EX_APP
EX_APP --|uses|--> INDEX
EX_HTML --|loads|--> EX_MAIN
%% Configuration Dependencies
PKG --|configures|--> INDEX
TS --|configures|--> INDEX
PKG --|defines deps|--> PLYR
PKG --|defines deps|--> REACT
PKG --|defines deps|--> REACT_DOM
%% Styling
DIST_CSS --|styles|--> PLYR_COMP
EX_CSS --|styles|--> EX_APP
classDef sourceFile fill:#e1f5fe
classDef configFile fill:#f3e5f5
classDef buildFile fill:#e8f5e8
classDef exampleFile fill:#fff3e0
classDef dependency fill:#ffebee
classDef component fill:#e0f2f1
class INDEX,TYPES sourceFile
class PKG,TS,README,GIT configFile
class DIST_ESM,DIST_CJS,DIST_CSS,DIST_TYPES buildFile
class EX_PKG,EX_HTML,EX_MAIN,EX_APP,EX_CSS exampleFile
class PLYR,REACT,REACT_DOM dependency
class PLYR_COMP,USE_PLYR,PLYR_CLASS component
Repository Architecture OverviewThis diagram shows the complete structure of the plyr-react repository, which is a React wrapper library for the Plyr media player. Here's what each section represents: 🏗️ Main Components
📁 Key Directories
🔗 Dependencies & Relationships
📦 DistributionThe library is packaged as:
This architecture supports the v6 breaking changes mentioned in the PR objectives, particularly the shift to named exports and ES module packaging. 🏁 Script executed: Length of output: 114953 🏁 Script executed: Length of output: 3575 @chintan9 Here's a comprehensive Mermaid diagram representing the entire plyr-react repository structure, relationships, and architecture: graph TB
subgraph "🏗️ plyr-react Repository (v6.0.0)"
subgraph "📁 Root Configuration"
PKG[package.json<br/>📦 Main package config<br/>v6.0.0-2]
TS_CONFIG[tsconfig.json<br/>⚙️ TypeScript config<br/>ES modules + strict mode]
ROLLDOWN[rolldown.config.ts<br/>🔧 Build configuration<br/>tsdown bundler]
ESLINT[eslint.config.js<br/>📏 Linting rules<br/>Latest ESLint config]
end
subgraph "📂 Source Code (src/)"
INDEX[index.tsx<br/>🎯 Main exports<br/>• Plyr component (named)<br/>• usePlyr hook<br/>• Types & interfaces]
TYPES_FILE[types.d.ts<br/>📝 Type definitions<br/>(intentionally empty)]
end
subgraph "📦 Distribution (dist/)"
DIST_ESM[index.js<br/>📦 ES Module build<br/>Tree-shakeable]
DIST_CJS[index.cjs<br/>📦 CommonJS build<br/>Legacy support]
DIST_TYPES[index.d.ts<br/>📝 Type declarations<br/>Generated from source]
DIST_CSS[plyr.css<br/>🎨 Plyr player styles<br/>From plyr package]
DIST_MAPS[*.map files<br/>🗺️ Source maps<br/>For debugging]
end
subgraph "🧪 Testing & Quality"
TESTS[tests/Plyr.test.tsx<br/>🧪 Unit tests<br/>Jest + Testing Library]
HUSKY[.husky/<br/>🪝 Git hooks<br/>pre-commit + pre-push]
LINT_STAGED[.lintstagedrc.json<br/>📋 Staged file linting]
end
subgraph "🎬 Example Application"
EX_ROOT[example/plyr-example/<br/>📁 Vite demo app]
EX_PKG[package.json<br/>📦 Example config<br/>React 19 + Vite 7]
EX_HTML[index.html<br/>🌐 HTML entry point]
EX_MAIN[src/main.jsx<br/>⚡ React app entry<br/>Vite integration]
EX_APP[src/App.jsx<br/>🎬 Demo component<br/>Shows plyr-react usage]
EX_CSS[src/App.css + index.css<br/>🎨 Example styling]
EX_VITE[vite.config.js<br/>⚙️ Vite configuration<br/>React plugin]
end
subgraph "🔗 External Dependencies"
REACT_APTOR[react-aptor<br/>🔧 Lifecycle manager<br/>v2.0.0]
PLYR_CORE[plyr<br/>🎥 Core media player<br/>v3.7.7+ (peer dep)]
REACT_LIB[react<br/>⚛️ React library<br/>>=16.8 (peer dep)]
TSDOWN[tsdown<br/>📦 Build tool<br/>Rolldown-based bundler]
end
subgraph "📋 Documentation & CI"
README[README.md<br/>📖 Project documentation<br/>Installation & usage]
CHANGELOG[CHANGELOG.md<br/>📜 Version history<br/>v6 breaking changes]
GITHUB[.github/<br/>🤖 GitHub workflows<br/>CI/CD automation]
end
end
%% Build Process Flow
INDEX --> |compiles to| DIST_ESM
INDEX --> |compiles to| DIST_CJS
INDEX --> |generates| DIST_TYPES
ROLLDOWN --> |configures| DIST_ESM
ROLLDOWN --> |configures| DIST_CJS
TS_CONFIG --> |configures| INDEX
%% Package Dependencies
PKG --> |defines| REACT_APTOR
PKG --> |peer dep| PLYR_CORE
PKG --> |peer dep| REACT_LIB
PKG --> |uses| TSDOWN
%% Component Architecture
INDEX --> |exports named| PLYR_COMP[Plyr Component<br/>ForwardRefExoticComponent]
INDEX --> |exports| USE_PLYR[usePlyr Hook<br/>Lifecycle management]
INDEX --> |exports types| PLYR_TYPES[PlyrInstance<br/>PlyrOptions<br/>PlyrSource<br/>PlyrProps]
%% Example Dependencies
EX_PKG --> |depends on| PKG
EX_APP --> |imports from| INDEX
EX_MAIN --> |renders| EX_APP
EX_HTML --> |loads| EX_MAIN
EX_VITE --> |configures| EX_MAIN
%% External Integration
USE_PLYR --> |uses| REACT_APTOR
PLYR_COMP --> |wraps| PLYR_CORE
REACT_APTOR --> |manages| PLYR_CORE
%% Testing & Quality
TESTS --> |tests| INDEX
HUSKY --> |validates| TESTS
LINT_STAGED --> |lints| INDEX
%% CSS & Styling
DIST_CSS --> |styles| PLYR_COMP
EX_CSS --> |styles| EX_APP
%% V6 Breaking Changes Indicators
INDEX -.->|❌ No default export| BREAKING1[Breaking: Named exports only]
DIST_ESM -.->|✅ ES modules first| BREAKING2[Breaking: Module system change]
PKG -.->|📦 New export map| BREAKING3[Breaking: Export structure]
classDef sourceFile fill:#e3f2fd,stroke:#1976d2,stroke-width:2px
classDef buildFile fill:#f3e5f5,stroke:#7b1fa2,stroke-width:2px
classDef configFile fill:#e8f5e8,stroke:#388e3c,stroke-width:2px
classDef exampleFile fill:#fff3e0,stroke:#f57c00,stroke-width:2px
classDef dependencyFile fill:#ffebee,stroke:#d32f2f,stroke-width:2px
classDef exportFile fill:#e0f2f1,stroke:#00796b,stroke-width:2px
classDef breakingChange fill:#ffcdd2,stroke:#c62828,stroke-width:3px,stroke-dasharray: 5 5
class INDEX,TYPES_FILE sourceFile
class DIST_ESM,DIST_CJS,DIST_TYPES,DIST_CSS,DIST_MAPS buildFile
class PKG,TS_CONFIG,ROLLDOWN,ESLINT,LINT_STAGED configFile
class EX_ROOT,EX_PKG,EX_HTML,EX_MAIN,EX_APP,EX_CSS,EX_VITE exampleFile
class REACT_APTOR,PLYR_CORE,REACT_LIB,TSDOWN dependencyFile
class PLYR_COMP,USE_PLYR,PLYR_TYPES exportFile
class BREAKING1,BREAKING2,BREAKING3 breakingChange
Repository Architecture OverviewThis diagram represents the complete plyr-react v6 architecture, highlighting the major changes and structure: 🏗️ Core Architecture
🔄 v6 Breaking Changes
🎬 Example Integration
🔗 Dependencies & Architecture
📦 Distribution Strategy
This architecture reflects a modern, maintainable approach to React component libraries with proper separation of concerns and professional build tooling. |
|
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (7)
rolldown.config.ts (1)
15-26: Fix hardcoded CSS path and add graceful error handling.The hardcoded
node_modules/plyr/dist/plyr.csspath is brittle in workspaces/PNPM and will throw if the file is missing. This issue was flagged in a previous review.Apply this diff to use Node's resolver and handle missing files gracefully:
import fs from "fs"; import path from "path"; import { fileURLToPath } from "url"; +import { createRequire } from "module"; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); +const require = createRequire(import.meta.url); const copyCssPlugin = { name: "copy-css", writeBundle() { - const source = path.resolve(__dirname, "node_modules/plyr/dist/plyr.css"); const destinationDir = path.resolve(__dirname, "dist"); const destinationFile = path.join(destinationDir, "plyr.css"); fs.mkdirSync(destinationDir, { recursive: true }); - fs.copyFileSync(source, destinationFile); - console.log("✅ Copied plyr.css to dist/plyr.css"); + try { + const source = require.resolve("plyr/dist/plyr.css"); + fs.copyFileSync(source, destinationFile); + console.log("✅ Copied plyr.css to dist/plyr.css"); + } catch (err) { + console.warn("⚠️ plyr/dist/plyr.css not found. Is 'plyr' installed as a peer dependency?"); + } }, };README.md (1)
66-71: Align the plyr peer dependency version with the example.The peer dependency version listed here (3.7.7) is inconsistent with the example which uses ^3.8.3. This was flagged in a previous review.
Apply this diff to align the documentation:
-- **`plyr`**: Version `3.7.7` or a compatible version. +- **`plyr`**: Version `^3.8.3` or newer compatible version.package.json (3)
19-19: Fix CSS tree-shaking: mark CSS files as having side effects.With
"sideEffects": false, bundlers will remove CSS imports likeimport "plyr-react/plyr.css"during tree-shaking. This was flagged in a previous review.Apply this diff:
- "sideEffects": false, + "sideEffects": ["*.css", "dist/plyr.css"],
37-43: Add a prepublishOnly guard to ensure fresh builds.The "release" script can publish stale or missing builds. This was flagged in a previous review.
Apply this diff:
"scripts": { "build": "tsdown --config rolldown.config.ts --minify", "dev": "tsdown --config rolldown.config.ts --watch", "lint": "eslint --fix '{src,tests}/**/*.{ts,tsx}'", "test": "jest", "prepare": "husky install", - "release": "npm publish" + "release": "npm publish", + "prepublishOnly": "npm run build" },
54-75: Pin ESLint/TS-ESLint dependencies and update Node engine requirement.Multiple dependencies are set to "latest", which can cause unexpected build breakages. Additionally, ESLint v9 and @typescript-eslint v8 both require Node ≥18.18.0, but the engines field specifies Node ≥16. This was flagged in a previous review.
Based on learnings: ESLint 9.33.0 and typescript-eslint 8.x require Node 18.18+.
Apply this diff to pin dependencies and update the Node engine:
"devDependencies": { "@swc/core": "^1.3.42", "@swc/jest": "^0.2.24", "@testing-library/jest-dom": "^5.16.5", "@testing-library/react": "^12.1.5", "@types/jest": "^27.5.2", "@types/react": "^18.0.28", - "@typescript-eslint/eslint-plugin": "latest", - "@typescript-eslint/parser": "latest", - "eslint": "latest", - "eslint-config-prettier": "latest", + "@typescript-eslint/eslint-plugin": "^8.0.0", + "@typescript-eslint/parser": "^8.0.0", + "eslint": "^9.33.0", + "eslint-config-prettier": "^9.0.0", "eslint-import-resolver-typescript": "^4.4.4", - "eslint-plugin-import": "latest", - "eslint-plugin-jest": "latest", - "eslint-plugin-prettier": "latest", - "eslint-plugin-react": "latest", - "eslint-plugin-react-hooks": "latest", + "eslint-plugin-import": "^2.29.0", + "eslint-plugin-jest": "^27.0.0", + "eslint-plugin-prettier": "^5.0.0", + "eslint-plugin-react": "^7.35.0", + "eslint-plugin-react-hooks": "^5.0.0", "husky": "^8.0.3", "jest": "^27.5.1", "jest-environment-jsdom": "^27.5.1", "lint-staged": "^12.5.0", "plyr": "^3.7.7", - "prettier": "latest", + "prettier": "^3.0.0", "react": "^17.0.2", "react-dom": "^17.0.2", "tsdown": "^0.2.0", "typescript": "^5.0.4", "typescript-eslint": "^8.41.0" },And update the engines field at lines 80-82:
"engines": { - "node": ">=16" + "node": ">=18.18.0" },Security note: eslint-config-prettier had a supply-chain compromise in mid-2025. Ensure you install a clean, post-remediation version and verify npm deprecation notices. Based on learnings.
src/index.tsx (2)
11-19: ESM interop: use default import; avoid* as+.default.The namespace import +
.defaultis fragile across ESM/CJS and test environments. Use a default import and export types directly.-// This is the critical change for the import -import * as Plyr from "plyr"; +import PlyrJS from "plyr"; @@ -// And this is the critical change for usage -const PlyrJS = Plyr.default; - -export type PlyrInstance = Plyr.default; -export type PlyrOptions = Plyr.Options; -export type PlyrSource = Plyr.SourceInfo; +export type PlyrInstance = PlyrJS; +export type { Options as PlyrOptions, SourceInfo as PlyrSource } from "plyr";
36-44: Bug: selector-based instantiation binds the wrong element. Use the provided node.Using
new PlyrJS(".plyr-react", ...)targets the first match globally and breaks multiple instances. Instantiate with the actual element argument.-const instantiate: Instantiate< - PlyrInstance, - HTMLVideoElement, - PlyrConfigurationProps -> = (_, params) => { - const plyr = new PlyrJS(".plyr-react", params?.options ?? {}); +const instantiate: Instantiate< + PlyrInstance, + HTMLVideoElement, + PlyrConfigurationProps +> = (el, params) => { + const plyr = new PlyrJS(el, params?.options ?? {}); if (params?.source) plyr.source = params?.source; return plyr; };
🧹 Nitpick comments (7)
example/plyr-example/eslint.config.js (1)
16-24: Consider simplifying the ecmaVersion configuration.The
ecmaVersionis set in bothlanguageOptions(2020) andparserOptions("latest"). While not incorrect, this is redundant. You can remove thelanguageOptions.ecmaVersionand rely onparserOptions.ecmaVersion: "latest"for simplicity.Apply this diff to simplify:
languageOptions: { - ecmaVersion: 2020, globals: globals.browser, parserOptions: { ecmaVersion: "latest", ecmaFeatures: { jsx: true }, sourceType: "module", }, },README.md (1)
262-262: Fix heading level for consistency.The "Acknowledgments and Key Contributors" heading jumps from h2 (Support) to h4 without an intermediate h3. Recommend changing to h3 for consistent heading hierarchy.
Apply this diff:
-#### Acknowledgments and Key Contributors +### Acknowledgments and Key ContributorsBased on static analysis hints.
example/plyr-example/README.md (1)
1-12: Consider adding plyr-react specific documentation.This README contains standard Vite template content but lacks plyr-react specific setup or usage instructions. Consider adding a section documenting how to use plyr-react in this example.
example/plyr-example/src/index.css (1)
70-70: Remove the redundant inline comment.The comment
/* src/index.css */at line 70 is redundant since it appears within the index.css file itself. Remove it during consolidation of the duplicate declarations.eslint.config.js (1)
15-17: Enable type-checked rules and set parserOptions.project.You’re only using the non‑typed preset. Prefer the type‑checked config and enable project detection for TS files.
- // Base configurations - ...tseslint.configs.recommended, + // Base configurations (use type-checked rules) + ...tseslint.configs.recommendedTypeChecked, @@ { files: ["**/*.{ts,tsx}"], + // Enable type-aware linting by pointing to a tsconfig (auto-detect nearest) + languageOptions: { + parserOptions: { project: true }, + }, plugins: { import: importPlugin, react: reactPlugin, "react-hooks": reactHooksPlugin, },Also applies to: 20-41
src/index.tsx (2)
104-117: Breaking change: named export only. Consider dual export or clearly document migration.Dropping the default export will break consumers. Either add a default export alongside the named export, or ensure v6 release notes and README include migration steps.
-export { PlyrComponent as Plyr }; +export { PlyrComponent as Plyr }; +export default PlyrComponent;
54-67: Proxy fallback masks runtime mistakes; prefer explicit “not ready” handling.Casting to
APITypesand returning a Proxy that no‑ops can hide bugs. Consider returning{ plyr: null }viaAPITypes | nullor throwing on access until ready.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (25)
dist/LICENSEis excluded by!**/dist/**dist/README.mdis excluded by!**/dist/**dist/esm/index.d.tsis excluded by!**/dist/**dist/esm/index.jsis excluded by!**/dist/**dist/esm/index.mjsis excluded by!**/dist/**dist/index.cjsis excluded by!**/dist/**dist/index.cjs.mapis excluded by!**/dist/**,!**/*.mapdist/index.d.ctsis excluded by!**/dist/**dist/index.d.tsis excluded by!**/dist/**dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.mapdist/package.jsonis excluded by!**/dist/**dist/plyr.cssis excluded by!**/dist/**dist/system/index.development.jsis excluded by!**/dist/**dist/system/index.production.jsis excluded by!**/dist/**dist/umd/index.development.jsis excluded by!**/dist/**dist/umd/index.production.jsis excluded by!**/dist/**example/nextjs/public/favicon.icois excluded by!**/*.icoexample/nextjs/public/vercel.svgis excluded by!**/*.svgexample/nextjs/yarn.lockis excluded by!**/yarn.lock,!**/*.lockexample/plyr-example/package-lock.jsonis excluded by!**/package-lock.jsonexample/plyr-example/public/vite.svgis excluded by!**/*.svgexample/plyr-example/src/assets/react.svgis excluded by!**/*.svgexample/react/yarn.lockis excluded by!**/yarn.lock,!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (48)
.eslintignore(0 hunks).eslintrc.json(0 hunks).github/workflows/nodejs.yml(1 hunks).husky/pre-push(1 hunks).lintstagedrc.json(1 hunks)README.md(3 hunks)babel.config.js(0 hunks)eslint.config.js(1 hunks)example/nextjs/.eslintrc.json(0 hunks)example/nextjs/.gitignore(0 hunks)example/nextjs/README.md(0 hunks)example/nextjs/next.config.js(0 hunks)example/nextjs/package.json(0 hunks)example/nextjs/pages/_app.js(0 hunks)example/nextjs/pages/api/hello.js(0 hunks)example/nextjs/pages/index.js(0 hunks)example/nextjs/src/contents.json(0 hunks)example/nextjs/src/video-list.jsx(0 hunks)example/nextjs/styles/globals.css(0 hunks)example/plyr-example/.gitignore(1 hunks)example/plyr-example/README.md(1 hunks)example/plyr-example/eslint.config.js(1 hunks)example/plyr-example/index.html(1 hunks)example/plyr-example/package.json(1 hunks)example/plyr-example/src/App.css(1 hunks)example/plyr-example/src/App.jsx(1 hunks)example/plyr-example/src/index.css(1 hunks)example/plyr-example/src/main.jsx(1 hunks)example/plyr-example/vite.config.js(1 hunks)example/react/README.md(0 hunks)example/react/package.json(0 hunks)example/react/src/audio-player/custom-audio-player.tsx(0 hunks)example/react/src/hls-player/custom-hls-player.tsx(0 hunks)example/react/src/index.css(0 hunks)example/react/src/main.tsx(0 hunks)example/react/src/video-list/contents.json(0 hunks)example/react/src/video-list/video-list.tsx(0 hunks)example/react/src/vite-env.d.ts(0 hunks)example/react/tsconfig.json(0 hunks)example/react/vite.config.ts(0 hunks)jest.config.ts(0 hunks)package.json(1 hunks)rolldown.config.ts(1 hunks)rollup.config.js(0 hunks)src/index.tsx(5 hunks)src/types.d.ts(1 hunks)tests/Plyr.test.tsx(2 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (27)
- example/nextjs/styles/globals.css
- example/react/src/video-list/video-list.tsx
- example/react/src/vite-env.d.ts
- example/nextjs/.gitignore
- example/react/src/main.tsx
- example/react/tsconfig.json
- example/react/README.md
- example/react/src/hls-player/custom-hls-player.tsx
- .eslintrc.json
- example/react/src/audio-player/custom-audio-player.tsx
- example/nextjs/pages/_app.js
- example/react/vite.config.ts
- example/react/src/video-list/contents.json
- example/nextjs/pages/index.js
- rollup.config.js
- example/nextjs/package.json
- jest.config.ts
- example/nextjs/pages/api/hello.js
- example/react/package.json
- example/nextjs/next.config.js
- example/nextjs/src/video-list.jsx
- example/nextjs/README.md
- example/nextjs/.eslintrc.json
- .eslintignore
- example/react/src/index.css
- babel.config.js
- example/nextjs/src/contents.json
🧰 Additional context used
🧬 Code graph analysis (7)
tsconfig.json (2)
babel.config.js (1)
api(1-24)rollup.config.js (1)
input(44-59)
rolldown.config.ts (1)
rollup.config.js (6)
input(30-42)input(61-73)input(44-59)c(125-136)target(22-28)targets(15-20)
package.json (2)
rollup.config.js (1)
input(44-59)babel.config.js (1)
api(1-24)
example/plyr-example/package.json (3)
example/react/src/video-list/video-list.tsx (2)
video(13-23)VideoList(10-26)rollup.config.js (1)
createUMDConfig(75-102)example/nextjs/src/video-list.jsx (1)
VideoList(10-25)
example/plyr-example/src/App.jsx (3)
example/react/src/video-list/video-list.tsx (2)
video(13-23)VideoList(10-26)example/react/src/audio-player/custom-audio-player.tsx (1)
props(22-53)example/nextjs/src/video-list.jsx (1)
VideoList(10-25)
src/index.tsx (4)
dist/index.d.ts (6)
PlyrInstance(5-5)PlyrOptions(6-6)PlyrSource(7-7)APITypes(14-16)PlyrComponent(19-19)PlyrProps(13-13)dist/index.d.cts (6)
PlyrInstance(5-5)PlyrOptions(6-6)PlyrSource(7-7)APITypes(14-16)PlyrComponent(19-19)PlyrProps(13-13)example/plyr-example/src/App.jsx (1)
ref(23-23)example/react/src/audio-player/custom-audio-player.tsx (1)
props(22-53)
README.md (2)
example/react/src/audio-player/custom-audio-player.tsx (1)
props(22-53)example/react/src/video-list/video-list.tsx (1)
video(13-23)
🪛 markdownlint-cli2 (0.18.1)
README.md
60-60: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
262-262: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
🔇 Additional comments (19)
src/types.d.ts (1)
1-2: LGTM!The removal of the global
__DEV__declaration aligns with the PR's cleanup of legacy tooling. The comments clearly document the file's current purpose.example/plyr-example/vite.config.js (1)
1-15: LGTM!The Vite configuration is correctly set up for React with appropriate Gitpod preview support via HMR on port 443 and allowedHosts filtering.
example/plyr-example/package.json (1)
1-29: LGTM!The example package.json is correctly configured with Vite tooling and appropriate dependencies. The
"plyr-react": "next"reference is expected for a v6 pre-release example.rolldown.config.ts (1)
29-36: LGTM!The tsdown configuration is correctly set up with appropriate entry points, formats, and plugins.
README.md (3)
1-44: LGTM!The updated title, introduction, and feature list provide a comprehensive and professional overview of the library's capabilities.
95-208: LGTM!The usage guide is comprehensive and well-structured, with clear examples for both basic and advanced use cases. The breaking change note for the CSS import path is particularly helpful for users upgrading from v4 to v5.
209-257: LGTM!The examples, demo links, nightly version reference, contribute, and support sections are clear and comprehensive.
package.json (2)
20-35: LGTM!The module type and dist-based exports configuration is correctly structured for ESM/CJS dual format and TypeScript support.
76-114: LGTM!The peer dependencies, publishConfig, and inline Jest configuration are correctly structured. The Jest global
__DEV__setting aligns with the removal of the global type declaration insrc/types.d.ts.example/plyr-example/index.html (1)
1-13: LGTM!The HTML template is correctly configured for a Vite + React application with appropriate favicon, title, and script entry point.
.husky/pre-push (1)
4-4: LGTM! Enhanced pre-push validation.The hook now enforces a full validation pipeline (lint → build → test) before push, which improves code quality gates.
example/plyr-example/src/App.jsx (2)
4-4: LGTM! Correctly uses the new named export.The import statement correctly uses the named export
{ Plyr }from plyr-react, aligning with the v6 migration from default to named exports.
7-15: Provider value 'youtube' is correct and must be lowercase as per Plyr docs.example/plyr-example/src/App.css (1)
1-42: LGTM!The CSS provides appropriate styling for the example app with accessibility considerations (reduced-motion support).
tests/Plyr.test.tsx (2)
3-3: LGTM! Correctly migrated to named exports.The import statement correctly uses named exports
{ Plyr, PlyrInstance }, aligning with the v6 API changes.
42-42: Good TypeScript practice improvement.Switching from
@ts-ignoreto@ts-expect-erroris better because it will fail if the suppressed error no longer exists, preventing stale suppressions..lintstagedrc.json (1)
2-3: LGTM! More explicit and comprehensive lint configuration.The change:
- Adds JS/JSX linting alongside TS/TSX
- Separates code files (linted + formatted) from non-code files (formatted only)
- Replaces the generic "*" pattern with explicit patterns, improving maintainability
example/plyr-example/src/main.jsx (1)
6-6: CSS import path is valid. package.json exports include./plyr.css→./dist/plyr.css, anddist/plyr.cssexists.eslint.config.js (1)
45-46: Test file glob is correct. All test files live under tests/ with .tsx extensions, sotests/**/*.{ts,tsx}already includes them.
| "import/resolver": { | ||
| typescript: true, | ||
| node: true, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
Ensure import resolvers are installed/configured.
eslint-plugin-import resolution needs resolvers. Verify eslint-import-resolver-typescript and eslint-import-resolver-node are in devDependencies to avoid “unable to resolve” warnings.
🏁 Script executed:
#!/bin/bash
# Check required resolvers and plugin versions
jq -r '.devDependencies, .dependencies' package.json | jq -s 'add' | jq -r '
{
import: .["eslint-plugin-import"],
resolver_ts: .["eslint-import-resolver-typescript"],
resolver_node: .["eslint-import-resolver-node"],
react: .["eslint-plugin-react"],
react_hooks: .["eslint-plugin-react-hooks"],
jest: .["eslint-plugin-jest"],
prettier_cfg: .["eslint-config-prettier"]
}'Length of output: 172
Install missing eslint-import-resolver-node
Add eslint-import-resolver-node to devDependencies to satisfy the node: true resolver and prevent “unable to resolve” warnings.
Example:
npm install --save-dev eslint-import-resolver-node🤖 Prompt for AI Agents
In eslint.config.js around lines 36 to 40, the ESLint import resolver config
enables node but the project is missing the eslint-import-resolver-node package;
add eslint-import-resolver-node to devDependencies (e.g., run npm install
--save-dev eslint-import-resolver-node or yarn add -D
eslint-import-resolver-node) and commit the updated package.json and
package-lock.json/yarn.lock so the resolver no longer emits “unable to resolve”
warnings.
| }, | ||
|
|
||
| // Prettier configuration (must be last) | ||
| prettierConfig |
There was a problem hiding this comment.
🧩 Analysis chain
Security: use a safe eslint-config-prettier version.
There were compromised releases mid‑2025. Ensure you’re on a post‑remediation version.
Based on learnings
🏁 Script executed:
#!/bin/bash
# Show installed version and npm deprecation notices
jq -r '.devDependencies["eslint-config-prettier"] // .dependencies["eslint-config-prettier"] // "not-found"' package.json
npm view eslint-config-prettier time | tail -n 15
npm view eslint-config-prettier deprecated || trueLength of output: 565
Pin eslint-config-prettier to ≥10.1.6
Replace "latest" in package.json with a specific post-remediation version (e.g. 10.1.8) to avoid installing the compromised 10.1.3–10.1.5 releases.
🤖 Prompt for AI Agents
In eslint.config.js at line 55, the project currently relies on an
unspecified/“latest” release of eslint-config-prettier which risks pulling
compromised versions; update package.json to pin eslint-config-prettier to a
safe fixed version (e.g. "10.1.8" or any >=10.1.6), remove any "latest"
specifier, run npm/yarn install to update node_modules and regenerate the
lockfile (package-lock.json or yarn.lock), and verify linting works and CI
passes.
| body { | ||
| margin: 0; | ||
| display: flex; | ||
| place-items: center; | ||
| min-width: 320px; | ||
| min-height: 100vh; | ||
| } |
There was a problem hiding this comment.
Duplicate body declarations with conflicting properties.
The file contains two body rule blocks:
- Lines 25-31: Sets
margin: 0,display: flex,place-items: center, min dimensions - Lines 71-78: Sets
font-family,background-color,color,display: flex,justify-content: center,padding
Properties like display, background-color, and layout will cascade/override. Consolidate these into a single body declaration to avoid confusion and ensure intended styles apply.
Apply this diff to consolidate the body declarations:
body {
margin: 0;
display: flex;
- place-items: center;
+ justify-content: center;
+ align-items: center;
min-width: 320px;
min-height: 100vh;
+ font-family: system-ui, sans-serif;
+ background-color: #1a1a1a;
+ color: white;
+ padding: 2rem;
}
h1 {
font-size: 3.2em;
line-height: 1.1;
}
...
-/* src/index.css */
-body {
- font-family: system-ui, sans-serif;
- background-color: #1a1a1a;
- color: white;
- display: flex;
- justify-content: center;
- padding: 2rem;
-}
-
.App {
max-width: 800px;
width: 100%;
text-align: center;
}Also applies to: 71-78
🤖 Prompt for AI Agents
In example/plyr-example/src/index.css around lines 25-31 (and also lines 71-78),
there are duplicate body rule blocks with conflicting layout and visual
properties; consolidate them into a single body declaration by merging the
needed properties (margin, display, place-items/justify-content,
min-width/min-height, font-family, background-color, color, padding) into one
block, remove the other duplicate, and ensure the final combined order reflects
intended cascade (keep any vendor-specific or fallback values first and more
specific/overriding values last).
| ### System Requirements | ||
|
|
||
| - **Node.js:** Version `16` or higher. | ||
| - **React:** Version `16.8` or higher. | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Update Node.js requirement to match ESLint/TypeScript tooling.
The stated requirement (Node.js 16+) is inconsistent with the project's devDependencies (ESLint 9.x and @typescript-eslint 8.x both require Node >=18.18.0). This should be updated to reflect the actual minimum supported version.
Apply this diff once package.json engines field is updated:
-- **Node.js:** Version `16` or higher.
+- **Node.js:** Version `18.18.0` or higher.This aligns with the past review comment on package.json lines 54-75 and 81-82 regarding Node engine requirements.
🏁 Script executed:
#!/bin/bash
set -e
echo "DevDependencies from package.json:"
jq '.devDependencies' package.json
# ESLint
ESLINT_SPEC=$(jq -r '.devDependencies.eslint' package.json)
RESOLVED_ESLINT=$(npm view "eslint@$ESLINT_SPEC" version)
echo "ESLint spec: $ESLINT_SPEC -> resolved version: $RESOLVED_ESLINT"
npm view "eslint@$RESOLVED_ESLINT" engines
# @typescript-eslint/parser
PARSER_SPEC=$(jq -r '.devDependencies["@typescript-eslint/parser"]' package.json)
RESOLVED_PARSER=$(npm view "@typescript-eslint/parser@$PARSER_SPEC" version)
echo "@typescript-eslint/parser spec: $PARSER_SPEC -> resolved version: $RESOLVED_PARSER"
npm view "@typescript-eslint/parser@$RESOLVED_PARSER" engines
# @typescript-eslint/eslint-plugin
PLUGIN_SPEC=$(jq -r '.devDependencies["@typescript-eslint/eslint-plugin"]' package.json)
RESOLVED_PLUGIN=$(npm view "@typescript-eslint/eslint-plugin@$PLUGIN_SPEC" version)
echo "@typescript-eslint/eslint-plugin spec: $PLUGIN_SPEC -> resolved version: $RESOLVED_PLUGIN"
npm view "@typescript-eslint/eslint-plugin@$RESOLVED_PLUGIN" engines
echo "Current engines field in package.json:"
jq '.engines' package.jsonLength of output: 1274
Align Node.js requirement to ≥18.18.0
ESLint 9.36.0 and @typescript-eslint 8.45.0 require Node.js ≥18.18.0. Update package.json engines.node and the README.
--- package.json
- "engines": { "node": ">=16" }
+ "engines": { "node": ">=18.18.0" }### System Requirements
- **Node.js:** Version `16` or higher.
+ **Node.js:** Version `18.18.0` or higher.📝 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.
| ### System Requirements | |
| - **Node.js:** Version `16` or higher. | |
| - **React:** Version `16.8` or higher. | |
| "engines": { "node": ">=18.18.0" } |
| ### System Requirements | |
| - **Node.js:** Version `16` or higher. | |
| - **React:** Version `16.8` or higher. | |
| ### System Requirements | |
| - **Node.js:** Version `18.18.0` or higher. | |
| - **React:** Version `16.8` or higher. |
🤖 Prompt for AI Agents
In README.md around lines 88 to 92, the Node.js requirement is outdated (Node.js
16); update the text to require Node.js >=18.18.0 to match
ESLint/@typescript-eslint constraints. Also update package.json's "engines.node"
field to ">=18.18.0" (and any CI configs or docs that reference Node.js version)
so the codebase, CI, and README stay consistent.
v6
Summary by CodeRabbit
Breaking Changes
New Features
Documentation
Chores