Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possibility to provide pre-compilation functions.. #36

Closed
wants to merge 1 commit into from

Conversation

Dzhakhar
Copy link

@Dzhakhar Dzhakhar commented Mar 18, 2017

Possibility to provide pre-compilation functions as a props. If you would like to modify the match.url and/or match.text before it's insertion, you can just provide the callbacks:

  • preCompileURL
  • preCompileText
<Linkify preCompileURL={ (url) => {
    return window.location.protocol + '//' + window.location.host + '/redirectTo?url=' + url
}}>
Hello, World! This is my website http://website.com
</Linkify>

Result:

Hello, World! This is my website <a href="https://github.com/redirectTo?url=http://website.com">
 http://website.com
</a>

Possibility to provide pre-compilation functions as a props. If you would like to modify the `match.url` and/or `match.text` before it's insertion, you can just provide the callbacks:

- `preCompileURL`
- `preCompileText`

```<Linkify preCompileURL={ (url) => {return window.location.protocol + '//' + window.location.host + '/redirectTo?url=' url} }>
Hello, World! This is my website http://website.com
</Linkify>```

Result:
```<a>Hello, World! This is my website https://github.com/redirectTo?url=http://website.com</a>```
Copy link

@peterbe peterbe left a comment

Choose a reason for hiding this comment

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

I have my doubts. Perhaps we can do something bigger and better that is future proof.

For example, I might prefer target="_blank" iff the URL matches someRegex.test(match.url).

Perhaps the simplest solution is to make the properties prop a callable.

E.g.

<Linkify properties={p => {
   console.log(p.href)  // logs 'https://www.github.com'
   console.log(p.text) // logs 'www.github.com'
   if (/someregex/.test(p.href)) {
      p.href += '?foo=bar'
      p.target = '_blank'
   } else if (/otherregex/.test(p.text)) {
       someGlobal()
   }
   return p
}}>Some text here www.github.com thing...</Linkify>

That means we don't need to have a specific solution for href and a specific solution for text plus it makes it possible to extend/modify the properties in runtime.

What do you think @tasti ?

I know it's just a nit but the convention @tasti uses is to put a space after the word if.

let text = match.text;

if(this.props.preCompileText){
text = this.props.preCompileText(match.text);
Copy link

Choose a reason for hiding this comment

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

I think this should be this.props.preCompileText(match.text, props.href) too so that you can re-write the text based on information of the URL.

emailRegex: React.PropTypes.object
emailRegex: React.PropTypes.object,
preCompileURL: React.PropTypes.func,
preCompileText: React.PropTypes.func
Copy link

Choose a reason for hiding this comment

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

Why the word "compile"?
To me, that word means you're preparing something before you can use it. Also, the word "pre" is nice but it's jarring to me that there's no "post".

props = {href: this.props.preCompileURL(match.url), key: `parse${this.parseCounter}match${idx}`};
} else {
props = {href: match.url, key: `parse${this.parseCounter}match${idx}`};
}
Copy link

Choose a reason for hiding this comment

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

Here you're repeating yourself. I think something like this would be better:

let url = match.url
if (this.props.preCompileURL) {
    url = this.props.preCompileURL(url)
}
let props = {href: url, key: `parse${this.parseCounter}match${idx}`};

Copy link
Author

Choose a reason for hiding this comment

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

Of course, i realized it later but forgot to fix

@peterbe
Copy link

peterbe commented Mar 21, 2017

By the way, my personal usecase is something like this:

<Linkify properties={p => {
   const siteTitle = syncLookupSiteTitle(p.href)
   if (siteTitle) {
     p.title = `Original URL: ${p.href}`
     p.text = siteTitle
  }
  return p
}}>...</Linkify>
  • Would be cool if that p in the pseudo code above could be a mutable so I don't have to return it.
  • Perhaps I'd like to do return false or return null to tell Linkify to NOT turn it into a hyperlink if/because of some very custom business logic.
  • Would be cool if I could use async to re-render that Linkify in an async callback.

@Dzhakhar
Copy link
Author

Dzhakhar commented Mar 21, 2017

@peterbe i think it would be nice to allow providing function that will mutate the props before compiling, you're right, this way is more flexible than mutating a particular parts of props

@tasti tasti mentioned this pull request Jan 6, 2018
16 tasks
@tasti
Copy link
Owner

tasti commented Jan 7, 2018

Support for pre-compilation of url and text will be available in v1.0.0. For example:

const hrefDecorator = (href) => ...;
const textDecorator = (text) => ...;

<Linkify hrefDecorator={hrefDecorator} textDecorator={textDecorator}>
  ...
</Linkify>

Any feedback is appreciated.

@tasti tasti closed this Jan 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants