iansinnott / react-string-replace

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

Chained calls to reactStringReplace #1

Closed Guibod closed 8 years ago

Guibod commented 8 years ago

Thanks for the library.

Is there any workaround to chain multiple calls to reactStringReplace ?

TypeError: First argument to react-string-replace must be a non-empty string

iansinnott commented 8 years ago

Hm, I'm not quite sure what you mean. Can you write out an example of what functionality you would like to see?

Guibod commented 8 years ago

Say I have a tweet to transform:

Hey @iansinnott, check this link https://github.com/iansinnott/ #github

I need to apply 3 distinct regexp on the string:

const reactStringReplace = require('react-string-replace')
const originalTweet = 'Hey @iansinnott, check this link https://github.com/iansinnott/ #github';
const linkedTweet = reactStringReplace(originalTweet, /https?:\/\/\S+/, (match, i) => (
  <a href={match}>{match}</span>
));
const mentionTweet = reactStringReplace(linkedTweet, /@\w+/, (match, i) => (
  <a href={`https://twitter.com/${match}`}>@{match}</span>
));
const hashTagTweet = reactStringReplace(mentionTweet, /#\w+/, (match, i) => (
  <a href={`https://twitter.com/hashtag/${match}?src=hash`}>#{match}</span>
));

(above code was not tested, that's may contains tons of errors)

I can still apply one ultra complex regexp to the whole string, and then do the magic inside the callback, but that's kind of messy. Don't you think ?

iansinnott commented 8 years ago

Ah I see, yeah that's a good point. Do you have any thoughts on what the API might look like for multiple matches?

One initial thought: An array of match/replacer tuples:

const reactStringReplace = require('react-string-replace')
const originalTweet = 'Hey @iansinnott, check this link https://github.com/iansinnott/ #github';
const linkedTweet = reactStringReplace(originalTweet, [
  [
    /https?:\/\/\S+/,
    (match, i) => (
      <a href={match}>{match}</a>
    )
  ], [
    /@\w+/,
    (match, i) => (
      <a href={`https://twitter.com/${match}`}>@{match}</a>
    )
  ], [
    /#\w+/,
    (match, i) => (
      <a href={`https://twitter.com/hashtag/${match}?src=hash`}>#{match}</a>
    )
  ],
]);

I'm not sure I like the array-of-arrays API, so I'm just thinking out loud here. As you said it is possible to create a regex that matches all three and then do the replacement logic in the single replacement function. I can see how that isn't ideal though.

Let me know if you have thoughts on what an ideal API would look like.

Guibod commented 8 years ago

I really don't know. Your input seems ok. Yet, it is bulky.

Could you just allow your function to accept both string and array of strings and react components ? And bypass components ?

iansinnott commented 8 years ago

Could you just allow your function to accept both string and array of strings and react components ? And bypass components ?

Could you elaborate with an example? I'm not sure what this would look like

Guibod commented 8 years ago

See my second message. Currently reactStringReplace requires a string and return an array of string and components.

I'm proposing you to allow an array of string and components as input, and apply reactStringReplace recursively on any string in the array, ignoring components in the process. Then output a flat array of string and components, just as if it was a strinq input.

iansinnott commented 8 years ago

Ah yes, I see what you mean. Good point :+1:

I'd happily welcome a PR if you want to add this functionality, but I will also think about how best to address this. It's definitely a current limitation that I'd like to fix.

iansinnott commented 8 years ago

This should be working now. I added some tests for the new functionality but please re-open if you run into any issues.

For details see 0e2a167.