Fix plugin idempotency issue with React re-renders (#14)

* 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: katspaugh/wavesurfer.js#3731

* Fix lint error: suppress unused variable warning for plugins destructuring

* 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

* Update lint

Author: katspaugh

.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"

.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

.gitignore

@@ -4,4 +4,7 @@ docs
 node_modules
 yarn-error.log
 .env
-examples/app.js
+examples/app.js
+coverage
+report.json
+eslint_report.json

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 (
+    <>
+      <WavesurferPlayer
+        height={100}
+        waveColor="violet"
+        url="/audio.wav"
+        plugins={plugins}
+      />
+      <div id="timeline" />
+    </>
+  )
+}
+```
+
+### 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 (
+    <>
+      <WavesurferPlayer
+        height={100}
+        waveColor="violet"
+        url="/audio.wav"
+        plugins={plugins}
+      />
+      <div id="timeline" />
+    </>
+  )
+}
+```
+
+### 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 (
+    <>
+      <WavesurferPlayer
+        height={100}
+        waveColor="violet"
+        url="/audio.wav"
+        plugins={plugins}
+      />
+      <div id="timeline" />
+    </>
+  )
+}
+```
+
 ## Docs
 
 https://wavesurfer.xyz

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": {},

src/index.tsx

@@ -38,15 +38,23 @@ function useWavesurferInstance(
   options: Partial<WaveSurferOptions>,
 ): WaveSurfer | null {
   const [wavesurfer, setWavesurfer] = useState<WaveSurfer | null>(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
 }

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)
+  })
 })

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"