fix: bundling compatibility with webpack@5 #2519
Conversation
when using webpack v5 to bundle code that calls `require('sinon')` (cjs) , it would have defaulted to "exports->require" and fail with multiple node-api requirements (util, timers, etc.)
this patch ensures that anyone who bundles sinon for browser gets the (browser-compatible) esm version.
tested on both webpack v5 and v4. should be noted that v4 worked even without this patch, as it automatically injected polyfills. v5 no longer does so. with this PR, people using webpack@4 to bundle sinon at least see size improvement, as the polyfills are no longer required.
|
Thanks, seems good! Needs a test run, of course :-) |
|
Looks like browserify doesn't like the esm in package.json->browser. I'll revert that one and leave the one in the exports. All should be well then. |
browserify doesn't seem to like esm. leave that entry point alone, and ensure "exports" -> "browser" (which webpack@5 uses) is esm.
|
Reverted. Tests should (hopefully) now pass. On a related note, please consider dropping browserify support... it's pretty outdated in 2023. |
|
@fatso83 heya, any chance we can rerun ci on the smaller change? |
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #2519 +/- ##
=======================================
Coverage 96.00% 96.00%
=======================================
Files 40 40
Lines 1900 1900
=======================================
Hits 1824 1824
Misses 76 76
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Now passes on CI. :) Would appreciate if this could land main, as it would allow us to remove workarounds to the bundling issues in several repositories. |
|
Ironically, this seems to have broken MUI's webpack setup: mui/material-ui#37430 (comment) 👀 |
|
Yeah, webpack now resolves the esm version, so this config: https://github.com/mui/material-ui/blob/master/test/regressions/webpack.config.js#LL26C1-L29C8 probably needs to now say They might even be able to remove the ProvidePlugin completely if sinon was the only place needing this polyfill. |
Purpose (TL;DR)
Ensure sinon bundles nicely for browsers without any errors or warnings.
#2411 re-fixed (after "exports" were added I think).
Background
When using webpack v5 to bundle cjs code that calls
require('sinon'), it would have defaulted to "exports->require" and fail with multiple node API errors/warnings (util, timers, etc.).This patch ensures that anyone who bundles sinon with a bundler that respects "exports" gets the (browser-compatible) esm version.
Tested on both webpack v5 and v4. should be noted that v4 worked even without this patch, as it automatically injected polyfills. webpack@5 no longer does so.
Solution
The esm version bundles without any errors/warnings, so make sure bundlers pick that one up.
How to verify
Use the same steps to reproduce in #2411
Modify the node_modules with this patch and rerun
npx webpack --mode developmentChecklist for author
npm run lintpasses