Skip to content

Conversation

@katspaugh
Copy link
Owner

Problem

Fixes katspaugh/wavesurfer.js#3731

The plugin instances were being mutated during initialization by wavesurfer.js, and when the React component re-rendered (especially in strict mode with double rendering), the same mutated plugin instances were passed to a new wavesurfer instance, causing errors.

Solution

Modified the useWavesurferInstance hook to:

  1. Exclude plugins from deep dependency comparison - Created optionsWithoutPlugins that separates plugins from other options
  2. Track plugin array reference separately - Added options.plugins directly to the useEffect dependency array, so the instance only recreates when the plugins array reference changes (not when individual plugins are mutated)
  3. Pass plugins correctly - Spread optionsWithoutPlugins and explicitly pass plugins: options.plugins to ensure plugins are included

Key Benefits

  • ✅ Plugins work correctly in React strict mode (double render)
  • ✅ Plugin instances are not re-initialized on every render
  • ✅ Users can memoize the plugins array with useMemo() to prevent unnecessary wavesurfer re-creation
  • ✅ All existing tests pass
  • ✅ Added comprehensive plugin test to verify the fix

Changes

  • Modified src/index.tsx to handle plugins separately in dependency tracking
  • Added test case for plugin re-render behavior
  • Updated README.md with detailed plugin usage examples and memoization requirements (with bold warning that plugins must be memoized)

Testing

All tests pass including the new plugin test:

✓ should properly cleanup on unmount
✓ should render wavesurfer with basic options
✓ should render wavesurfer with events
✓ should handle plugins correctly and not break on re-render

The fix ensures plugin idempotency even when React strict mode causes double 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
@katspaugh
Copy link
Owner Author

CI Status Update

The CI checks are showing failures, but these are due to GitHub Actions infrastructure/permissions issues, not code issues:

✅ Lint Check

Passes locally with no errors. CI failure is due to artifact upload permissions:

Error: Create Artifact Container failed: The artifact name eslint-v8-cache-key-state-Lint is not valid

✅ Tests Check

All tests pass (4/4 including the new plugin test). CI failure is due to missing checks: write permission:

Tests:       4 passed, 4 total
##[error]Missing checks: write permission

Local Verification

All checks pass locally:

  • npm run lint - No errors
  • npm test - 4/4 tests pass
  • npm run build - Builds successfully

The code changes are ready for review. The CI failures are unrelated to this PR.

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-actions
Copy link

github-actions bot commented Dec 4, 2025

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
63.89% (+11.03% 🔼)
46/72
🟡 Branches
63.64% (+27.27% 🔼)
14/22
🔴 Functions
56.25% (+4.64% 🔼)
18/32
🟢 Lines 100% 1/1

Test suite run success

4 tests passing in 1 suite.

Report generated by 🧪jest coverage report action from e199f76

@katspaugh katspaugh merged commit a85b89c into main Dec 4, 2025
4 checks passed
@katspaugh katspaugh deleted the fix/plugin-idempotency branch December 4, 2025 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Plugins initialise twice and break in react wrapper

2 participants