YahooArchive / fluxible-router

MOVED TO FLUXIBLE REPO
104 stars 26 forks source link

navlink performance improvement, provide createNavLink, remove handleRoutes #70

Closed kaesonho closed 9 years ago

kaesonho commented 9 years ago

Higher order component pattern is good but since users might use NavLink everywhere, so having connectToStore -> handleRoute -> NavLink -> a should be too much and the performance might be able to be improved.

  1. We only use makePath and isActive from routeStore, use it directly, so that we can reduce two higher component and we can remove the eventListeners to the RouteStore. previously the RouteStore might propagate hundreds emitChange to every NavLink on the page.
  2. move out NavLink functions to be navLinkUtils, so that if users have the use case they can compose there own NavLink. provide createNavLink so that users can overwrite the attribute with whatever they want, maybe add a mixin or a custom click event handler

@mridgway @redonkulus @lingyan @Vijar

kaesonho commented 9 years ago

updated, now provide a function for users to overwrite attributes and create a NavLink component

kaesonho commented 9 years ago

updated, still listen to the RouteStore change.

yahoocla commented 9 years ago

CLA is valid!

mridgway commented 9 years ago

lgtm

kaesonho commented 9 years ago

thanks :fireworks:, squashed

redonkulus commented 9 years ago

:+1:

kaesonho commented 9 years ago

do we want to put createNavLink in lib ? or addons ?

mridgway commented 9 years ago

Keep it in lib since it's used by core. We can export it from require('fluxible-router').createNavLinkComponent.

kaesonho commented 9 years ago

updated, change file name as createNavLinkComponent, also export it with index.js

lingyan commented 9 years ago

:+1:

mridgway commented 9 years ago

:boat:

kaesonho commented 9 years ago

rebased, let me merge it

roderickhsiao commented 9 years ago

:+1: