Skip to content

Remove strip-ansi dependency#474

Open
roli-lpci wants to merge 1 commit intowebpack:mainfrom
roli-lpci:chore/remove-strip-ansi
Open

Remove strip-ansi dependency#474
roli-lpci wants to merge 1 commit intowebpack:mainfrom
roli-lpci:chore/remove-strip-ansi

Conversation

@roli-lpci
Copy link
Copy Markdown

Summary

  • Replace strip-ansi with an inline regex equivalent
  • Removes 1 runtime dependency (strip-ansi + its transitive dep ansi-regex)

Changes

client.js

  • Replace require('strip-ansi') with an inline strip() function using the
    same ANSI escape regex from ansi-regex v5 (the version used by strip-ansi v6)
  • Since client.js runs in the browser (bundled by webpack), the native Node.js
    util.stripVTControlCharacters API cannot be used here

package.json

  • Remove strip-ansi from dependencies

Testing

  • ESLint: passes (with scoped no-control-regex disable for the ANSI regex literal)
  • Prettier: passes
  • Mocha tests: 1 pre-existing flaky failure in realistic multi compiler (reproduces on main)

Addresses #465

Replace the strip-ansi package with an inline regex equivalent.
The ANSI stripping regex is the same pattern used by strip-ansi v6,
inlined to eliminate the dependency.

Since client.js runs in the browser (bundled by webpack), the native
Node.js util.stripVTControlCharacters API cannot be used here.

Addresses webpack#465

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@evenstensberg evenstensberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to keep strip ansi. Anything related to regex's is a security sink

@glenjamin
Copy link
Copy Markdown
Collaborator

I agree, this is a simple dependency that does one thing well, there is little to be gained by inlining it

@alexander-akait
Copy link
Copy Markdown
Member

@evenstensberg There is a good example how our issue PR is ignored, we should look how to disable it (due AI generated)

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.

4 participants