React-router: Why children expect a function and component expect a component on v4?

[gartz]

I know this is a design decision, and it looks like, was made to be closer to the previous version. But it really makes less readable to create components this way.

The children of components that match could accept component and render it if match.

The component can give other properties that accept functions, this is closer to what react does.

Example:

<Match pattern="/foo">
  <div>Print this</div>
</Match>

Still, would work with function callbacks to transport props.

To render when it matches or not, it could be a prop, which only accepts function.

Please look this two examples:

function InitialPage() {}
function Page() {}
function NotFound() {}

function complexLogic({ path }) {
  if (path.page === 'foo') {
    return (<Page />)
  }
  return (<NotFound />);
}

function Main() {
  return (
    <div>
      <Match pattern="/" exactly component={ InitialPage } />
      <Match pattern="/:page">{ complexLogic }</Match>
    </div>
  );
}

It looks like the previous version, but when going nested levels become harder, because you need to separate components all the time, even to show a simple div in some case.

function Main() {
  return (
    <div>
      <Match pattern="/" exactly>
        <div>I can write a simple initial page here.</div>
        <InitialPage />
      </Match>
      <Match pattern="/:page" onMatch={ complexLogic } />
    </div>
  );
}

I hope it's not too late to consider that. By using this beta, I hit that many times and the second example always looked more logical to me, I can go deeper and show a more cool thing we can do with the second example:

function PageA() {
  return (
    <Match pattern="pageA">
      <div> Only display if typed /pageA in the url</div>
    </Match>
  );
}

function PageB() {
  return (
    <Match pattern="pageB">
      <div> Only display if typed /pageB in the url</div>
    </Match>
  );
}

function Main() {
  return (
    <div>
      <Match pattern="/" exactly>
        <div>I can write a simple initial page here.</div>
        <InitialPage />
      </Match>
      <Match pattern="/:page">
        <PageA />
        <PageB />
        <Miss> Ooops, no page found... </Miss>
      </Match>
    </div>
  );
}

Now, it would display pages on the second match, but each page is self-aware of it URL fragment, and a miss would be able to go after, this design is very attractive for modular pages.

It's possible to do something like it with the current spec, but it's not as elegant as this.

Please let me know your thoughts.

[ryanflorence]

https://gist.github.com/ryanflorence/a301dc184f75e929a263dc1e80399a28
On Fri, Dec 9, 2016 at 4:36 PM Gabriel Reitz Giannattasio <
[email protected]> wrote:

I know this is a design decision, and it looks like, was made to be closer
to the previous version. But it really makes less readable to create
components this way.

The children of components that match could accept component and render
it if match.

The component can give other properties that accept functions, this is
closer to what react does.

Example:

Print this

Still, would work with function callbacks to transport props.

To render when it matches or not, it could be a prop, which only accepts
function.

Please look this two examples:

function InitialPage() {}function Page() {}function NotFound() {}
function complexLogic({ path }) { if (path.page === 'foo') { return ()
} return ();
}
function Main() { return (

{ complexLogic }

);
}

It looks like the previous version, but when going nested levels become
harder, because you need to separate components all the time, even to show
a simple div in some case.

function Main() { return (

I can write a simple initial page here.

);
}

I hope it's not too late to consider that. By using this beta, I hit that
many times and the second example always looked more logical to me, I can
go deeper and show a more cool thing we can do with the second example:

function PageA() { return (

Only display if typed /pageA in the url

);
}
function PageB() { return (

Only display if typed /pageB in the url

);
}
function Main() { return (

I can write a simple initial page here.

Ooops, no page found...

);
}

Now, it would display pages on the second match, but each page is
self-aware of it URL fragment, and a miss would be able to go after, this
design is very attractive for modular pages.

It's possible to do something like it with the current spec, but it's not
as elegant as this.

Please let me know your thoughts.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/ReactTraining/react-router/issues/4268, or mute the
thread
https://github.com/notifications/unsubscribe-auth/AAGHaLiX3xgo1srIvTCfTOZUri_NB31Aks5rGcnngaJpZM4LJYch
.

[gartz]

@ryanflorence sorry to say, your analysis on how React works is wrong.

Using a conditional: https://jsfiddle.net/69z2wepo/64786/
Using a function: https://jsfiddle.net/69z2wepo/64787/

As you can see in the tests, constructor and render will be triggered the exactly same number of times on both approaches. Please reconsider this design choice.

[timdorr]

@gartz He's not referring to the rendering of that component, only the loading of that code to potentially render that code. The issue is something that comes up with code splitting and performance optimization. The control flow isn't the issue here. That's why Match works that way: to encourage good performance.

[gartz]

I remember to read about that in the react-router previous version discussions over isomorphic rendering and fetching data. And the same thing was said there applies here, the way fetch is being done or bundle is being loaded, are not a responsibility of the router.

I can make this implementation be async for loading bundles, and keep the same way to write.

Look this example:

class LoadPage {
  getAsyncBundleData(pathname) {
    // Some code that interact with withRouter and will lazy load my components, while it's not ready
    // When the promise is of my bundle is resolved I will forceUpdate
    // I can add any complex kind of script to load my bundles, pre-load, cancel the request, whatever.
  }

  render() {
    this.getAsyncBundleData(this.props.path);

    // Allow Miss to work properly
      return <Match pattern={ this.props.path }>{
        this.state[this.router.pathname]
        ? this.state[this.router.pathname]
        : <div>Loading...</div>
    }</Match>;
  }
}

const LoadPageWithRouter = withRouter(LoadPage);

function Main() {
  return (
    <div>
      <Match pattern="/" exactly>
        <div>I can write a simple initial page here.</div>
        <InitialPage />
      </Match>
      <Match pattern="/:page">
        <LoadPageWithRouter path="pageA" />
        <LoadPageWithRouter path="pageB" />
        <Miss> Ooops, no page found... </Miss>
      </Match>
    </div>
  );
}

[gartz]

@timdorr after thinking more deeply on your statement and from @ryanflorence I didn't found indeed any benefit or good reason to only accept functions.

If you accept components, functions still can be used:

<Match pattern="/foo">{ myFuctionReturnNullOrComponent() }</Match>

With the example, you can do the previous code (more complete and complex example) to implement a lazy load.

You might argue that is technically same as:

<Match pattern="/foo">{ myFuctionReturnNullOrComponent }</Match>

But it's different because I might want to display something as default and a holder while I'm loading my bundle like:

<Match pattern="/foo">
  <h1>My partial page title</h1>
  <div>{ lazyLoadContentHereOrDisplayLoadingComponent() }</div>
</Match>

I really hope to help you guys improve your project, if you don't change while is beta, probably this change will only happen on the next major version and will be a breaking compatibility change again :/