Add Context component to enable single point of integration with react-router

[mattapperson]

Do something like:

<EUIProjectContext options={{
  onNavigation={href => this.props.history.push(href)}
}}>
  <EuiLink href="http://www.elastic.co">
     Elastic website
  </EuiLink>
</EUIProjectContext>

EuiLink and other linkable components could then just call onNavigation from context if it exists.

This way:

• every link, tab, step, and button does not need to be wrapped
• The pattern allows for additional wrapping/bindings as EUI grows

[mattapperson]

Idea pulled from PR #810 into it's own issue

[chandlerprall]

@cjcenizal responding to your comment on #810, this is indeed what I was starting to suggest.

Comparing the two approaches:

Idea 1

From #810 , something inline that generates an onClick for EuiLink e.g.

<Adapter to="/location">
  {(props) => (
    <EuiLink {...props}>click here</EuiLink>
  )}
</Adapter>

pros

• full control over each individual link is given to the developer building the link
• flexibility for links to work however they're intended to
• explicit generation of values (href, onClick) within immediate view of their use

cons

• repetitive
• with great power comes great responsibility; full control everywhere may not be desired

Idea 2

Provide a navigation handler via React context and make EuiLink aware of the interface.

<EUIProjectContext options={{
  onNavigation={href => this.props.history.push(href)}
}}>
  <EuiLink href="http://www.elastic.co">
     Elastic website
  </EuiLink>
</EUIProjectContext>

pros

• setup navigation interface once per application
• KISS/DRY for consumers of EuiLink

cons

• layer of indirection
• potential for a developer to not realize they're missing the router integration
• EUI is now complected with a configuration setup that may or may not be present
  • must have sane fallbacks
  • precedent is now set / easy to add more configuration items
  • unit tests with context = blerrrrgh 😞

[chandlerprall]

With those pros/cons in mind I lean toward CJ's original idea without context. Unless the mental and physical overhead of including the wrapping structure everywhere is a reasonably large burden, which could then have enough value to add the layer of indirection via context. I don't have the experience within Kibana code to make a value call there. As a developer I would probably find approach 1 to be mildly annoying at best.

[mattapperson]

Some feedback on your pro/con list

For the one con of “potential to not realize they don’t have context” I would say this is perhaps not a bad thing? I mean it would be easy to debug the lack of the parent context.

As for sane fallback, if we just call the context method if it exists, sane fallback would just be to not call it. Works the same as now.

The indirection is perhaps the biggest con here imho and I agree completely that it is... non-ideal

[chandlerprall]

For the one con of “potential to not realize they don’t have context” I would say this is perhaps not a bad thing? I mean it would be easy to debug the lack of the parent context.

Easy to debug, but how difficult to notice you need to (honest question, I don't have a good answer)?

As for sane fallback, if we just call the context method if it exists, sane fallback would just be to not call it. Works the same as now.

Totally! Simple in this case, but it does add more logic to the component.

One thought I just had - can plugins (now or in the future) inject links in places?

• it would be nice for plugin links to Just Work™
• external links are not [well] supported by React Router, the project's official stance is external links should use plain anchor tags instead of the Link component (at least it used to be this way, may have changed... I can't find definitive documentation)

[mattapperson]

Easy to debug, but how difficult to notice you need to

I would say easy. if you add a page, and the link reloads your screen, then you need the context component. I think a simple note in the docs would suffice for this. But perhaps thats just me? Would love feedback on that from others.

can plugins (now or in the future) inject links in places?

"Inject links" no, support for linking between plugins I think is something that is desired AFAIK but not via injection.

for external links, yes RR does not really support them, I have always just used window.location in the callback for this... the onNavigation could easily do this if/when needed

[chandlerprall]

@jen-huang @nreese @pugnascotia any thoughts / feedback on usability around this vs. the approach in #810 ? (see OP & first comment on this thread for context/overview).

[nreese]

cc @stacey-gammon @chrisronline @bmcconaghy

[chrisronline]

I don't really understand what we're talking about here. What problem are we trying to solve exactly? Is it being able to translate a named route within a routing library to the actual url for use with EUILinks?

[chandlerprall]

Wanting to connect EuiLink into a routing system (e.g. react-router). Currently, <EuiLink href="/some/path"> creates an anchor element that navigates the browser to /some/path when it is clicked which reloads the entire application, instead of going to the routing system for navigation. There needs to be some layer that can connect EuiLink's anchors into routing.

The two approaches solve this by 1. explicitly wrapping EuiLink when you want to connect it to the routing system or 2. having the application define a page-wide navigation util that EuiLink knows how to transparently connect into.

[chrisronline]

We've just been doing <EuiLink href={`#${BASE_PATH}my_specific_path`}> where BASE_PATH is the base routing path to our plugin. Does that assume too much, like assuming on # routing? Does this not work for everyone?

[chrisronline]

@chandlerprall and I chatted offline about this and I'll offer my two cents.

I don't think adding an abstraction for handling programmatic routing adds much if any value. I honestly don't know any other react routing libraries other than react-router but I imagine each one provides a way to handle programmatic routing that works in all React environments. For react-router@v4, the most straight-forward approach is to wrap the component in the withRouter HoC then use the history prop added to manually navigate.

I'm doing this in my current project:

• Router configuration
• HoC
• Programmatic navigation

This feels natural to me as I'm relying on the specific routing library I'm using to handle the navigation and if I come in new to the project with experience of react-router, I'd expect to see this and it would make sense.

If we add an abstraction on top of this to handle all routing libraries, or avoid having to wrap with a HoC, it's going to make it harder to go from understanding how the routing library handles it to how EUI handles it.

Maybe I'm alone, but I don't see any reason to adjust my current approach for handling routing but I can be convinced otherwise :)

[cjcenizal]

@chrisronline Thanks for sharing your experience and the links to your implementation. I think it's really valuable to have a real use case added to the discussion and I think the way you're doing things is fine.

If we add an abstraction on top of this to handle all routing libraries, or avoid having to wrap with a HoC, it's going to make it harder to go from understanding how the routing library handles it to how EUI handles it.

I think this is a great point, and it adds more weight to going with Idea 1 above, which won't actually be added as code to EUI but will instead be provided as guidance in our documentation (#810). We could also document an alternative approach modeled after how you're doing things, though I'm guessing react-router's documentation already covers this approach since it looks pretty idiomatic.

[mattapperson]

@chrisronline You make some excellent points. At this point I am fine with closing this in favor of idea 1

[chandlerprall]

Closing this, moving back to #810. Thanks everyone!