<Link> Component performance

[khakulov]

Im my actual project i have to render a table with 300-400 rows. Every row contains a link to another page in my app. In actual beta4 version every rerendering of my table takes 400-500ms. Master brach is much better but still first rendering is to slow.

I looked deep in code by commenting out line by line the render function of Link component and realized that history.isActive takes to long to check active state of the link. Which state i don't need in my table.

The other small issue what i think is, that every link has onClick EventBindling. For every EventBinding browser allocates a small piece of memory. Which is no problem if the page has few links.

So i solved that problem for me, with normal html Tags and old school jQuery solution one global EventBinding on document which catches every link click and calls pushState if its realtive url (without http://):

$(document).delegate('a', 'click', evt => {
        if (evt.isPropagationStopped()) {
            return;
        }
        if(clickedWithoutHoldingAnyKeyboardButton(evt) && targetIsThisWindow(evt.target)) {
            let href = $(evt.target).attr("href");
            if (!isAbsoluteURL(href)) {
                evt.preventDefault();
                let [path, query] = parsePath(href);
                history.navigate({}, path, query);
            }
        }
});

[koulmomo]

React already delegates all events

[mjackson]

history.isActive takes to long to check active state of the link. Which state i don't need in my table

I think this is the real problem, @khakulov. isActive is an expensive function call.

Maybe we could work around this with a more "pure" <Link> component? Think <PureLink>. A <PureLink> wouldn't care about isActive at all and would just get it as a prop, which defaults to false.

[ryanflorence]

Its not the onClick event, that's free in react.

Until we can figure out how to make links checking if they're active faster, we should provide a link that doesn't care if its active for these types of scenarios.

[mjackson]

Perhaps the majority of links don't care if they're active or not? Literally the only use case I can think of is tabs/navigation.

[th0r]

Maybe just turn this behavior off by setting activeClass to null?

[th0r]

And use this value as default.

[knowbody]

yeah I agree, until we find out how we can optimise active links we could have <Link> without isActive and something like <NavLink> with the isActive behaviour. They are pretty much only used for navigation/tabs

[mjackson]

Maybe just turn this behavior off by setting activeClass to null?

That's a brilliant idea @th0r! Let's do that!

[AndrewIngram]

Can we really call this closed? A workaround was found, but that workaround is to not have active states, which doesn't feel like a solution at all. I'm experiencing terrible performance due to this issue, and needing active states.

I have 18 links (which do actually need active states), adding roughly 700ms to rendering times.

[timdorr]

@AndrewIngram Opening up a new issue with a fresh test case would be helpful. This is for a pre-1.0 version of the library, so the discussion here may not be relevant with all the changes that have been made over time. If you have suggestions for improving performance, please let us know in that new issue too.

[taion]

Nothing's really changed here; this is essentially unfixable for the general <Link> use case unless we want to drop nested routes with absolute paths (which IMO are a bit of a wart anyway).

#3158 will let us go back to doing something much faster for indexOnly isActive checks, though – and IMO indexOnly would be a better default anyway.