Skip to content

fix(extract-comments): decode Buffer sources before merging - #731

Merged
alexander-akait merged 1 commit into
webpack:mainfrom
tlaurie-cl:fix/extract-comments-buffer-source
Sep 9, 2026
Merged

fix(extract-comments): decode Buffer sources before merging#731
alexander-akait merged 1 commit into
webpack:mainfrom
tlaurie-cl:fix/extract-comments-buffer-source

Conversation

@tlaurie-cl

@tlaurie-cl tlaurie-cl commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #730.
Merging extracted comments into a file another plugin emitted calls .split("\n\n") on its source(), which is string | Buffer. If the source is a Buffer, it crashes the build with prevSource.source.split is not a function.

The fix checks whether source() actually returned a string or Buffer, and decodes it with toString("utf8") when it's a Buffer. The /** @type {string} */ assertion is removed.

This crash first appears in 5.9.0, but the /** @type {string} */ assertion is older: it was added in terser-webpack-plugin #381, when the merge only ever saw files this plugin itself had written, which were always strings. #716 then started merging into comment files that other plugins had already emitted, causing Buffers to newly reach .split(). webpack 5.110.3 depends on this plugin, so a project can hit the crash without depending on it directly.

What kind of change does this PR introduce?

A fix.

Did you add tests for your changes?

Yes, one new test: should work with the existing licenses file, when it is a Buffer.
The new test fails on main with the TypeError from the issue and passes with this change.

Does this PR introduce a breaking change?

No.

If relevant, what needs to be documented once your changes are merged or what have you already documented?

None needed.

Use of AI

Cursor's Auto Intelligence mode was used with supervision to debug the issue, trace the regression, build the reproduction and fix branches, and draft the bug and PR. All submission output and code has been reviewed and cleaned up by myself.

@linux-foundation-easycla

linux-foundation-easycla Bot commented Sep 8, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: tlaurie-cl / name: Thomas Laurie (47a31e2)

@changeset-bot

changeset-bot Bot commented Sep 8, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 47a31e2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
minimizer-webpack-plugin Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@tlaurie-cl
tlaurie-cl marked this pull request as draft September 8, 2026 22:10
@tlaurie-cl
tlaurie-cl force-pushed the fix/extract-comments-buffer-source branch 3 times, most recently from 4f7af04 to 723bc21 Compare September 8, 2026 22:17
@tlaurie-cl tlaurie-cl changed the title fix(extract-comments): handle Buffer sources, remove string type-assertion fix(extract-comments): decode Buffer sources before merging Sep 8, 2026
An existing comments file emitted by another plugin can be backed by a
Buffer, which has no split method. Decode it as UTF-8 before merging.
@tlaurie-cl
tlaurie-cl force-pushed the fix/extract-comments-buffer-source branch from 723bc21 to 47a31e2 Compare September 8, 2026 22:19
@tlaurie-cl
tlaurie-cl marked this pull request as ready for review September 8, 2026 22:43
@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 95.22%. Comparing base (649b48d) to head (47a31e2).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/index.js 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #731      +/-   ##
==========================================
- Coverage   95.27%   95.22%   -0.06%     
==========================================
  Files           4        4              
  Lines        1354     1360       +6     
  Branches      503      501       -2     
==========================================
+ Hits         1290     1295       +5     
- Misses         56       57       +1     
  Partials        8        8              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@alexander-akait
alexander-akait merged commit 139ff4e into webpack:main Sep 9, 2026
29 of 31 checks passed
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.

extractComments crashes when previous comments file is emitted as a Buffer

2 participants