From 94a3c3922df1567fb9d0538b82aae52f204fc556 Mon Sep 17 00:00:00 2001 From: katspaugh Date: Thu, 4 Dec 2025 20:01:34 +0100 Subject: [PATCH 1/4] Fix plugin idempotency issue with React re-renders - Exclude plugins from deep dependency comparison in useWavesurferInstance - Track plugins array reference separately to avoid re-initialization on mutation - Add comprehensive test for plugin re-render behavior - Update README with detailed plugin memoization examples and requirements Fixes: https://github.com/katspaugh/wavesurfer.js/issues/3731 --- README.md | 98 +++++++++++++++++++++++++++++++++++++++++++++ package.json | 2 +- src/index.tsx | 15 +++++-- tests/index.test.js | 47 ++++++++++++++++++++++ yarn.lock | 8 ++-- 5 files changed, 162 insertions(+), 8 deletions(-) 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..107f608 100644 --- a/package.json +++ b/package.json @@ -62,7 +62,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..768895b 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -38,15 +38,22 @@ 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(() => { + 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 +62,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" From 88fefaa4b2199117417b55bd8aab7680bbc9ffa6 Mon Sep 17 00:00:00 2001 From: katspaugh Date: Thu, 4 Dec 2025 20:04:18 +0100 Subject: [PATCH 2/4] Fix lint error: suppress unused variable warning for plugins destructuring --- src/index.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/index.tsx b/src/index.tsx index 768895b..566b69f 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -38,10 +38,11 @@ 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 // 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]) From 731896e98620c8a301ee51b5f61e4bb58a59d6aa Mon Sep 17 00:00:00 2001 From: katspaugh Date: Thu, 4 Dec 2025 20:08:01 +0100 Subject: [PATCH 3/4] Add checks:write and pull-requests:write permissions to workflows This fixes CI failures caused by missing permissions for GitHub Actions to: - Create check annotations - Post PR comments with test results and coverage reports --- .github/workflows/lint.yml | 4 ++++ .github/workflows/tests.yml | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index af9773e..a57977c 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -1,6 +1,10 @@ name: Lint on: [pull_request] +permissions: + checks: write + pull-requests: write + jobs: eslint: runs-on: ubuntu-latest 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 From e199f769ab845bd77558f8cd00324bb3f0b946dc Mon Sep 17 00:00:00 2001 From: katspaugh Date: Thu, 4 Dec 2025 20:14:38 +0100 Subject: [PATCH 4/4] Update lint --- .github/workflows/lint.yml | 12 +++++++++--- .gitignore | 5 ++++- package.json | 1 + 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index a57977c..99c5e18 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -9,12 +9,18 @@ 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/.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/package.json b/package.json index 107f608..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'",