Skip to content
This repository has been archived by the owner on Apr 29, 2023. It is now read-only.

Remove render prop in favor of render child #71

Closed
wants to merge 4 commits into from

Conversation

Swizz
Copy link
Contributor

@Swizz Swizz commented Apr 27, 2018

Now we have Lazy Components we can let user be responsible of using them to prevent computation issues.

Instead of using a render property, this PR add the ability to define route in a more composable way. Only the first child is used as rendered route.


<Route path="/">
  <Home/>
</Route>
Route({ path: "/" }, [Home])

<Route path="/old-match">
  {() => <Redirect from="/old-match" to="/will-match" />}
</Route>
Route({ path: "/old-match"}, [
  () => Redirect({ from: "/old-match", to: "/will-match" })
])

@Swizz Swizz added the BREAKING label Apr 27, 2018
@codecov
Copy link

codecov bot commented Apr 27, 2018

Codecov Report

Merging #71 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #71   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           6      6           
  Lines          68     68           
  Branches       11     11           
=====================================
  Hits           68     68
Impacted Files Coverage Δ
src/Route.js 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8db6747...312ef05. Read the comment docs.

@frenzzy
Copy link
Contributor

frenzzy commented Apr 27, 2018

Will it works with connected components?

const Example = (props) => (state) => (
  <h1>{JSON.stringify(state)}</h1> // { some, state } or { match, location } ?
)

@Swizz
Copy link
Contributor Author

Swizz commented Apr 27, 2018

Isn't how Lazy Component works ? The route will only trigger the Child Render (or Route View) function, using Lazy Components (or not) is up to you.

<Route path="/">
  { ({ match, location }) => <Example {...props}/> }
</Route>

We will basically have 3 kind of components :

Basics Components :

const Example = (props) => (
  <h1>{JSON.stringify(props)}</h1>
)

Lazy Components :

const Example = (props) => (state) => (
  <h1>{JSON.stringify(state)}</h1>
)

Routed Components :

const Example = (route) => (props) => (state) => (
  <h1>{JSON.stringify(route)}</h1>
)

@jorgebucaran jorgebucaran self-requested a review April 28, 2018 00:42
@znk
Copy link

znk commented Apr 28, 2018

How do you deal with subroutes?

@Swizz
Copy link
Contributor Author

Swizz commented May 2, 2018

@znk This PR is only about the render property. Subroutes will work the same.

Here is the new documentation

@znk
Copy link

znk commented May 23, 2018

@jorgebucaran I know that HA 2.x is coming soon, but can you status on this PR, please? It could be nice even for a minor version to seamlessly install it with npm/yarn from official repo. It is more declarative to write children that way. Without mentioning that it plays really well @hyperapp/transitions for page transition.

@jorgebucaran
Copy link
Owner

@znk In 2.0, children will be a prop of virtual nodes, so this pattern may everyone's new favorite.

@znk
Copy link

znk commented May 23, 2018

Possible to publish that version targeting HA 1.2.x ?

@jorgebucaran
Copy link
Owner

I want to hold that for 2.x. This is technically not blocking, so that's why I am not giving it a high priority.

@jorgebucaran
Copy link
Owner

I would be happy to merge a PR that removes lazy components from this implementation of the router.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
4 participants