iansinnott / react-string-replace

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

Cannot read property 'length' of undefined when used with regex #13

Closed mlepinay closed 7 years ago

mlepinay commented 7 years ago

I am trying to use your library this way to detect URLs in a string and reformat with some react components:

const LINK_DETECTION_REGEX = /(([a-z]+:\/\/)?(([a-z0-9\-]+\.)+([a-z]{2}|aero|arpa|biz|com|coop|edu|gov|info|int|jobs|mil|museum|name|nato|net|org|pro|travel|local|internal))(:[0-9]{1,5})?(\/[a-z0-9_\-\.~]+)*(\/([a-z0-9_\-\.%]*)(\?[a-z0-9+_\-\.%=&]*)?)?(#[a-zA-Z0-9!$&'()*+.=-_~:@\/?]*)?)(\s+|$)/gi;

formattedMessage = reactStringReplace(messageContent, LINK_DETECTION_REGEX, function(match, i) { 
    console.log(match)
    ...
})

The problem I have is that this regex makes the callback trigger 3 times with a standard URL such as: http://i.imgur.com/Pw4Xk7H.gif First time with match === "http://i.imgur.com/Pw4Xk7H.gif" (which is what I want) But then with match === "i.imgur.com" And again with match === "com"

After that it just crashes with the error:

Unhandled rejection TypeError: Cannot read property 'length' of undefined at replaceString (webpack:///./~/react-string-replace/index.js?:51:34)

Is it a bug? Is my regex wrong? (it was working with javascript replace)

Can someone investigate on this? Thanks

iansinnott commented 7 years ago

Hm, my guess is that this is an issue with the regex itself. It looks like you have a number of nested capture groups (i.e. everything within parens). Maybe try a different regex for just matching the URL or try using non-capturing matching groups: (?:...).

Your replacement function should be run for every match found in the regex, and since you match multiple nested strings of the top-level string the behavior you are seeing seems expected.

Try our regex out on https://regex101.com/ and see what you find.

mlepinay commented 7 years ago

I am in the end using a much simpler regex for the match. Then I am doing:

if (!LINK_DETECTION_REGEX.test(match)) {
    return '';
}

Which I am not super happy about. But it does the job for now.

iansinnott commented 7 years ago

OK, feel free to reopen if you have issues