Skip to content

Replaces first instance of matched string, not the actual matched regular expression #6

Description

@Conduitry

This is a bit tricky. I actually constructed this example after taking a quick look through this code, to confirm that what I thought was a bug was indeed a bug.

Suppose you have the string 'xaxb' and you're doing a replacement based on the regex /x(?!a)/g. That is, we're looking for all x that aren't immediately followed by an a. So this should match the second x and not the first x. If we use regular old .replace() we see that: 'xaxb'.replace(/x(?!a)/g, 'y') is 'xayb'.

If we do the same replacement with this library, though, the result is incorrect: 'yaxb'. When you use the String.prototype.match method to do the regex matching, all this returns to you is the array of matched strings - not where the matches occurred. Later, .indexOf() dutifully finds the first instance of 'x' in the string (which is all it has to go on), and this is eventually replaced. But this isn't what was actually matched or what should be replaced.

To avoid this bug (and also to avoid re-searching for the string, which is inefficient), you could instead use RegExp.prototype.exec, which will tell you, for each match, exactly where it occurred, which might not be the earliest instance of the matched string. This is the approach I took when was implementing my own (promise-based rather than callback-based) function for this, before I found yours.

Do whatever you'd like with this bug report! I'm not actually using your library, nor do I actually need this lookahead support.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions