Skip to content

fix: use regexp.escape#1439

Draft
ghostdevv wants to merge 4 commits intomainfrom
gd/regex-escape
Draft

fix: use regexp.escape#1439
ghostdevv wants to merge 4 commits intomainfrom
gd/regex-escape

Conversation

@ghostdevv
Copy link
Contributor

This switches us to use RegExp.escape() for escaping regex instead of custom regex or none at all.

TypeScript hasn't released the ES2025 changes that includes this typedef so I had to add it manually, not sure if there is a better place for the file?

@ghostdevv ghostdevv requested a review from danielroe February 12, 2026 18:28
@vercel
Copy link

vercel bot commented Feb 12, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Feb 13, 2026 3:19pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Feb 13, 2026 3:19pm
npmx-lunaria Ignored Ignored Feb 13, 2026 3:19pm

Request Review

@codecov
Copy link

codecov bot commented Feb 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 12, 2026

📝 Walkthrough

Walkthrough

Adds a TypeScript global augmentation declaring RegExp.escape(str: string) in global.d.ts and includes that file in Nuxt TypeScript config. Replaces direct interpolation into regexes with RegExp.escape(...) in useMarkdown.ts, useStructuredFilters.ts and shared/utils/emoji.ts. Also adds global.d.ts to knip's ignore list. No runtime logic changes beyond using escaped strings when building regular expressions.

Suggested reviewers

  • danielroe
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description clearly explains the changes: switching to RegExp.escape() for regex escaping and adding manual TypeScript definitions due to ES2025 support not being released yet.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch gd/regex-escape

No actionable comments were generated in the recent review. 🎉


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

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

I'm not sure that the benefit is worth the cost in terms of browser compatibility.

wdyt @ghostdevv?

@ghostdevv
Copy link
Contributor Author

ghostdevv commented Feb 12, 2026

I'm not sure that the benefit is worth the cost in terms of browser compatibility.

yeaaa maybe, perhaps there is a minimal ponyfill that achieves the same result 🤔 (because we need something for this, and I'd like to avoid having multiple fns for it over the codebase)

@ghostdevv
Copy link
Contributor Author

ghostdevv commented Feb 12, 2026

some initial findings, need to step away for a bit so will take another look later

@wojtekmaj
Copy link
Contributor

I'm not sure that the benefit is worth the cost in terms of browser compatibility.

wdyt @ghostdevv?

On the other hand, I don't think a large percentage of npmx audience is using IE or is 11 Chrome versions behind.

@danielroe
Copy link
Member

regexp.escape won't work on iOS 17 devices. I agree we need a canonical single escape function but it's a tiny one-liner, for our purposes. (https://github.com/lionel-rowe/regexp-escape-polyfill (linked by @ghostdevv above) is just 498 B)

@ghostdevv
Copy link
Contributor Author

updated to use @li/regexp-escape-polyfill

@ghostdevv
Copy link
Contributor Author

For some reason the vercel preview is crashing when you access a package page directly 🤔 it doesn't happen in dev or local preview

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.

3 participants