iansinnott / react-string-replace

A simple way to safely do string replacement with React components
MIT License
652 stars 56 forks source link

Not properly finds substrings if they are consecutive or at the end of the phrase #51

Closed jtomaszewski closed 4 years ago

jtomaszewski commented 4 years ago

Currently, following code:

function nl2br(content: string | undefined) {
  return reactStringReplace(content || '', /\n/g, (match, index) => (
    <React.Fragment key={index}>
      <br />
      {match}
    </React.Fragment>
  ));
}

const output = mount(<div>{nl2br('a\n\nb\nc\n')}</div>);
expect(output.html()).toEqual('<div>a<br><br>b<br>c<br></div>');

throws an error on the expect line:

Expected: "<div>a<br><br>b<br>c<br></div>"
Received: "<div>a<br>b<br>c</div>"

Which means, the \n\n gets replaced into only one <br /> instead of two; Also, the last \n in the didn't get replaced at all.

Also, why do I have to do this:

(match, index) => (
    <React.Fragment key={index}>
      <br />
      {match}
    </React.Fragment>
  ))

?

I'd expect from the documentation that

(match, index) => (
    <br key={index} />
  ))

would be enough, but then some text between the newlines get "swallowed" (it doesn't appear in the final string).

zappatic commented 4 years ago

Hi Jack, as the docs state, you have to have a matching group in your regex, which you don't. Try /(\n)/g instead of /\n/g and nothing will get swallowed. Your expect won't succeed because (at least in my test) it returned <br /> instead of <br>, but that is obviously a minor detail.

Also, no need to include {match} in the replacement function, your second example works fine (it would just re-add the \n's after the <br />'s).

jtomaszewski commented 4 years ago

You're right! Thanks.

Somehow I didn't notice it in the docs. (Or did I read them at all? Don't remember it now. Always read the docs first.)