Skip to content

fix(helpers): ignore wrapper characters in fromRegExp#3795

Merged
ST-DDT merged 7 commits intonextfrom
fix/helpers/fromRegExp/wrapper-chars
Apr 21, 2026
Merged

fix(helpers): ignore wrapper characters in fromRegExp#3795
ST-DDT merged 7 commits intonextfrom
fix/helpers/fromRegExp/wrapper-chars

Conversation

@ST-DDT
Copy link
Copy Markdown
Member

@ST-DDT ST-DDT commented Apr 7, 2026

Fixes #3663


Ignore the anchor characters at the start and end of regex patterns.

faker.helpers.fromRegExp(/^foo$/)
// old: ^foo$
// new: foo

@ST-DDT ST-DDT added this to the vAnytime milestone Apr 7, 2026
@ST-DDT ST-DDT self-assigned this Apr 7, 2026
@ST-DDT ST-DDT requested a review from a team as a code owner April 7, 2026 09:07
@ST-DDT ST-DDT added c: bug Something isn't working p: 1-normal Nothing urgent m: helpers Something is referring to the helpers module labels Apr 7, 2026
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 7, 2026

Deploy Preview for fakerjs ready!

Name Link
🔨 Latest commit 7ab4883
🔍 Latest deploy log https://app.netlify.com/projects/fakerjs/deploys/69e530f0d0ca4300085de70a
😎 Deploy Preview https://deploy-preview-3795.fakerjs.dev
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@ST-DDT ST-DDT requested review from a team and Copilot April 7, 2026 09:08
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.90%. Comparing base (c53c1fe) to head (7ab4883).
⚠️ Report is 3 commits behind head on next.

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #3795      +/-   ##
==========================================
- Coverage   98.90%   98.90%   -0.01%     
==========================================
  Files         897      897              
  Lines        3103     3102       -1     
  Branches      556      571      +15     
==========================================
- Hits         3069     3068       -1     
  Misses         30       30              
  Partials        4        4              
Files with missing lines Coverage Δ
src/modules/helpers/index.ts 95.27% <100.00%> (-0.02%) ⬇️
🚀 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.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates faker.helpers.fromRegExp to ignore start/end anchor characters (^ and $) when a RegExp object is provided, so anchored patterns don’t leak wrapper characters into the generated output (fixes #3663).

Changes:

  • Strip ^ / $ anchors from the extracted pattern when fromRegExp receives a RegExp.
  • Add a unit test asserting anchored /^foo$/i produces foo.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/modules/helpers/index.ts Adjusts the RegExp-to-pattern extraction to drop leading/trailing anchors.
test/modules/helpers.spec.ts Adds coverage for anchored input being unwrapped before generation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/modules/helpers/index.ts
Comment thread test/modules/helpers.spec.ts
@matthewmayer
Copy link
Copy Markdown
Contributor

this is arguably breaking, although seems doubtful anyone was relying on this?

@ST-DDT
Copy link
Copy Markdown
Member Author

ST-DDT commented Apr 7, 2026

The method basically only implements a third of all regex features. So we could declare this as wont't fix, but ^$ are common regex wrappers IMO..

Once the standalone module functions PR is merged, then I'll consider adding a faker-plugins-string-regexp2factory or similar package, that properly parses the regex into an AST (using a lib) in order to generate the output.

@hiSandog
Copy link
Copy Markdown
Contributor

The regex /\/\^?(.+?)\$?\/ correctly strips optional ^ and '$' anchors from the RegExp string representation.

One edge case to consider: if someone passes /^^foo$$/ (multiple anchors, which is valid regex but redundant), the current pattern would produce ^foo$ since ^? and $? only match zero or one occurrence. This is unlikely in practice, but using ^\^* and \$* (zero or more) would be more robust:

pattern = /\/\^*(.+?)\$*\//.exec(pattern)?.[1] ?? '';

Given that fromRegExp only implements a subset of regex features (as noted in the discussion above), the current fix is pragmatic for the common /^foo$/ case.

@ST-DDT
Copy link
Copy Markdown
Member Author

ST-DDT commented Apr 11, 2026

@faker-js/maintainers Thoughts?

@Shinigami92
Copy link
Copy Markdown
Member

@faker-js/maintainers Thoughts?

LGTM, this is a bug fix, not a breaking change - nobody should be relying on literal ^/$ leaking into the output.

suggestion: consider using pattern.source instead of parsing toString(), it gives you the pattern without slashes/flags directly:

if (pattern instanceof RegExp) {
  isCaseInsensitive = pattern.flags.includes('i');
  pattern = pattern.source.replace(/^\^/, '').replace(/\$$/, '');
}

a bit more readable and avoids the slash-parsing regex entirely.
Either way, happy to approve.

Shinigami92
Shinigami92 previously approved these changes Apr 17, 2026
@ST-DDT ST-DDT requested review from a team April 19, 2026 15:55
Comment thread src/modules/helpers/index.ts Outdated
@ST-DDT ST-DDT requested a review from a team April 19, 2026 19:45
@ST-DDT ST-DDT requested a review from a team April 19, 2026 19:45
@ST-DDT ST-DDT added this pull request to the merge queue Apr 21, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 21, 2026
@ST-DDT ST-DDT added this pull request to the merge queue Apr 21, 2026
Merged via the queue into next with commit 9e2c0e3 Apr 21, 2026
23 checks passed
@ST-DDT ST-DDT deleted the fix/helpers/fromRegExp/wrapper-chars branch April 21, 2026 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: bug Something isn't working m: helpers Something is referring to the helpers module p: 1-normal Nothing urgent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants