diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index af9773e..99c5e18 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -1,16 +1,26 @@ name: Lint on: [pull_request] +permissions: + checks: write + pull-requests: write + jobs: eslint: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Yarn shell: bash run: yarn install --immutable - - uses: Maggi64/eslint-plus-action@master + - name: Run lint + run: npm run lint:report + continue-on-error: true + + - name: Annotate code with linting results + uses: ataylorme/eslint-annotate-action@v3 with: - npmInstall: false + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + report-json: "eslint_report.json" diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index bd1e563..b002f54 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -1,6 +1,10 @@ name: Tests on: [pull_request] +permissions: + checks: write + pull-requests: write + jobs: tests: runs-on: ubuntu-latest diff --git a/.gitignore b/.gitignore index 1be4e3a..3adf168 100644 --- a/.gitignore +++ b/.gitignore @@ -4,4 +4,7 @@ docs node_modules yarn-error.log .env -examples/app.js \ No newline at end of file +examples/app.js +coverage +report.json +eslint_report.json \ No newline at end of file diff --git a/README.md b/README.md index 529e287..74affbe 100644 --- a/README.md +++ b/README.md @@ -91,6 +91,104 @@ const App = () => { } ``` +## Using plugins + +Wavesurfer [plugins](https://wavesurfer.xyz/docs/modules/plugins_index) can be passed in the `plugins` option. + +**Important:** The `plugins` array **must be memoized** using `useMemo` or defined outside the component. This is because wavesurfer.js mutates plugin instances during initialization, and passing a new array on every render will cause errors. + +### Basic example with a single plugin + +```js +import { useMemo } from 'react' +import WavesurferPlayer from '@wavesurfer/react' +import Timeline from 'wavesurfer.js/dist/plugins/timeline.esm.js' + +const App = () => { + const plugins = useMemo(() => { + return [ + Timeline.create({ + container: '#timeline', + }), + ] + }, []) + + return ( + <> + +
+ + ) +} +``` + +### Example with multiple plugins + +```js +import { useMemo } from 'react' +import WavesurferPlayer from '@wavesurfer/react' +import Timeline from 'wavesurfer.js/dist/plugins/timeline.esm.js' +import Regions from 'wavesurfer.js/dist/plugins/regions.esm.js' + +const App = () => { + const plugins = useMemo(() => { + return [ + Timeline.create({ + container: '#timeline', + }), + Regions.create(), + ] + }, []) + + return ( + <> + +
+ + ) +} +``` + +### Alternative: Define plugins outside the component + +If your plugins don't depend on component props or state, you can define them outside: + +```js +import WavesurferPlayer from '@wavesurfer/react' +import Timeline from 'wavesurfer.js/dist/plugins/timeline.esm.js' + +// Define plugins outside the component +const plugins = [ + Timeline.create({ + container: '#timeline', + }), +] + +const App = () => { + return ( + <> + +
+ + ) +} +``` + ## Docs https://wavesurfer.xyz diff --git a/package.json b/package.json index 3f87d6d..aea0a1f 100644 --- a/package.json +++ b/package.json @@ -27,6 +27,7 @@ "build": "rollup -c", "build:examples": "cd examples && rollup -c", "lint": "eslint src", + "lint:report": "eslint \"src/**/*.ts*\" --output-file eslint_report.json --format json", "test": "node --experimental-vm-modules node_modules/jest/bin/jest.js tests", "test:ci": "yarn test --ci --silent --coverage --json --watchAll=false --testLocationInResults --outputFile=report.json", "serve": "npx live-server --port=3030 --no-browser --ignore='.*,src,tests'", @@ -62,7 +63,7 @@ "rollup": "^3.26.2", "rollup-plugin-inject-process-env": "^1.3.1", "typescript": "^5.0.4", - "wavesurfer.js": "^7.9.4" + "wavesurfer.js": "^7.12.0" }, "jest": { "transform": {}, diff --git a/src/index.tsx b/src/index.tsx index 53512b1..566b69f 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -38,15 +38,23 @@ function useWavesurferInstance( options: Partial, ): WaveSurfer | null { const [wavesurfer, setWavesurfer] = useState(null) + // Flatten options object to an array of keys and values to compare them deeply in the hook deps - const flatOptions = useMemo(() => Object.entries(options).flat(), [options]) + // Exclude plugins from deep comparison since they are mutated during initialization + const optionsWithoutPlugins = useMemo(() => { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { plugins, ...rest } = options + return rest + }, [options]) + const flatOptions = useMemo(() => Object.entries(optionsWithoutPlugins).flat(), [optionsWithoutPlugins]) // Create a wavesurfer instance useEffect(() => { if (!containerRef?.current) return const ws = WaveSurfer.create({ - ...options, + ...optionsWithoutPlugins, + plugins: options.plugins, container: containerRef.current, }) @@ -55,7 +63,9 @@ function useWavesurferInstance( return () => { ws.destroy() } - }, [containerRef, ...flatOptions]) + // Only recreate if plugins array reference changes (not on mutation) + // Users should memoize the plugins array to prevent unnecessary re-creation + }, [containerRef, options.plugins, ...flatOptions]) return wavesurfer } diff --git a/tests/index.test.js b/tests/index.test.js index 8a8d695..b8fa982 100644 --- a/tests/index.test.js +++ b/tests/index.test.js @@ -65,4 +65,51 @@ describe('@wavesurfer/react tests', () => { expect(WaveSurfer.create).toHaveBeenCalled() }) + + it('should handle plugins correctly and not break on re-render', () => { + // Create a mock plugin that simulates being mutated during initialization + const mockPlugin = { + _init: jest.fn(), + _initialized: false, + init: function() { + if (this._initialized) { + throw new Error('Plugin already initialized') + } + this._initialized = true + this._init() + } + } + + const props = { waveColor: 'purple', plugins: [mockPlugin] } + + // Simulate the mock WaveSurfer.create calling plugin init + const originalMock = WaveSurfer.create.getMockImplementation() + WaveSurfer.create.mockImplementation((...args) => { + const instance = originalMock(...args) + // Simulate plugin initialization (wavesurfer.js would do this) + const options = args[0] + if (options.plugins) { + options.plugins.forEach(plugin => { + if (plugin.init) plugin.init() + }) + } + return instance + }) + + const { rerender } = render(React.createElement(WavesurferPlayer, props)) + + // First render should initialize the plugin + expect(mockPlugin._init).toHaveBeenCalledTimes(1) + expect(mockPlugin._initialized).toBe(true) + + // Re-render with the same props should not cause plugin re-initialization + // because plugins are now handled via ref + rerender(React.createElement(WavesurferPlayer, props)) + + // Plugin init should still only have been called once + expect(mockPlugin._init).toHaveBeenCalledTimes(1) + + // Restore original mock + WaveSurfer.create.mockImplementation(originalMock) + }) }) diff --git a/yarn.lock b/yarn.lock index acdf331..03bd535 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3363,10 +3363,10 @@ walker@^1.0.8: dependencies: makeerror "1.0.12" -wavesurfer.js@^7.8.11: - version "7.9.4" - resolved "https://registry.yarnpkg.com/wavesurfer.js/-/wavesurfer.js-7.9.4.tgz#fab2e52a4bf8e256f6f13dd85cdfed2bc7487d7d" - integrity sha512-ahOMvrOKo5jULNnXq8Ske8v/ZStoNNTDjYohvgLNerUFuh+6fJSt7wlxFesEXmnlcTnjMy5/tIzhn9KusjO6bg== +wavesurfer.js@^7.12.0: + version "7.12.0" + resolved "https://registry.yarnpkg.com/wavesurfer.js/-/wavesurfer.js-7.12.0.tgz#1b865af796a1cf7223122ad6d546411558c81ba3" + integrity sha512-/5PSaKkIC7PblJKCmkSfuFQK/k/VgAflaVIVtu+owj/NqyKn0p4ni6eFwZD6lfm66PdiPZrc5y9OZ/GDzkAS0Q== webidl-conversions@^7.0.0: version "7.0.0"