Props function in route definition should handle returning promises

[rhyek]

I'm creating an issue with my comments in #1126 since they never got any attention:

@posva, @Nirazul although your suggestions provide a solution, I think there is another way.

I like #973 very much because it leverages the use of props very nicely when trying to solve the problem of passing data from a route to a component in the most correct way. In my own issue, #992, I propose the option of passing props to a not-yet-created component during beforeRouteEnter. Currently, this hook only allows you to set data properties after the component has been created (next(vm => vm.someProp = resolvedData))

props are a way of providing bits of data to a component from an external source traditionally from a parent component or, less frequently, with propsData. Now, thanks to #973, it is also possible from a route definition. In all these cases, the data is available before a component is initialized.

Data coming over the wire is also an external source and, in my opinion and for consistency's sake, should be handled in a similar fashion. The most obvious way to do it would be to allow the props function in the route definition to optionally return a promise which, when resolved with the incoming data, allows us to continue with component creation passing along the data as props.

@posva:

{{ obj && obj.property }}

This is verbose and is a pattern I would like to not have to include in all my routable components. When you're designing a component that is meant to be called from a parent component, you don't have to do that because if you configure your props as required, you know that data is there at render time. Same should apply for API data.

<Spinner v-if="!obj"/>
<div v-else>
{{ obj.property }}
</div>

This is essentially what the data fetching section Fetching after navigation suggests and that's fine. This is how @Nirazul prefers it, too.

IMO:

• It is boilerplate I would not like to see repeated over and over in my application
• It doesn't work with props so I can't have validation
• It doesn't allow you to have things such as default handlers for API errors.
• No separation of concerns. My component should always just assume the data is there and all handling of errors or loading animations should be done in a more appropriate and probably centralized place.

I would like to be able to do something like this:

// router.js
const router = new VueRouter({
  routes: [
    {
      path: '/people/:id',
      component: require('components/Person'),
      props(to) {
        return axios
          .get(`/api/people/${ to.params.id }`)
          .then(response => response.data)
      }
    }
  ],
  errorHandlers: {
    301: (to, from, next, error) {
      next(error.response.headers.Location)
    },
    400: require('components/BadRequest'),
    401: () => {
      store.commit('logout') // store will navigate to home route or whatever
    },
    404: require('components/NotFound')
    // 400 and 404 will cause Vue to render their respective components
    // while optionally having already navigated to the new URL
  }
})

router.beforeEach(() =>  {
  store.commit('loading', true) // shows loading spinner
})

router.afterEach(() => {
  store.commit('loading', false) // hides loading spinner
})

<!-- Person.vue -->
<template>
  <div>
    {{ person.name }} <!-- no error -->
  </div>
</template>

<script>
export default {
  props: {
    person: {
      type: Object,
      required: true
    }
  }
}
</script>

<!-- BadRequest.vue -->
<template>
  <div>
    <h1>Oops!</h1>
    <p>{{ error.response.data.message }}</p>
  </div>
</template>

<script>
export default {
  props: {
    error: { // from promise catch()
      type: Object,
      required: true
    }
  }
}
</script>

Another concern I've seen here is what happens when a route param changes. IMO, you just start over from the moment you call props and either handle any errors or pass the resolved data to the component. Only difference is this time around the component is already initialized. Same thing that happens when a child component's props are modified in the parent component.

I realize this provides similar functionality to beforeRouteEnter and beforeRouteUpdate. Main differences are:

1. Using props vs data (data available before or after component initialization)
2. No need for two hooks, just one props function
3. Possibility of having global HTTP error handlers

[posva]

You're kind of overcomplicating things a lot... Especially with that errorHandler attr. So to put it in a nutshell, the problem is that you consider the route should have its data available upon mounting. This will indeed make things easier and prevent you from checking if an object exists before accessing its properties in the template. However, it doesn't allow you to control how the loading/fetching is handled at a view level, you can only handle it at a global level.

I personally use

<Spinner v-if="!obj"/>
<div v-else>
{{ obj.property }}
</div>

in my projects and you think it's boilerplate, but I disagree, because this allows you to customise what to display depending on the route, and gives you full flexibility.

The real problem here is what you pointed out in #992: It'll be great being to call next before mount, so writing obj.foo doesn't produce an error. Which is imo what we should fix instead

[rhyek]

@posva:

However, it doesn't allow you to control how the loading/fetching is handled at a view level, you can only handle it at a global level.

How many different ways to show a spinner can there be? To me it looks like boilerplate. Anyways, with the proposed change, you could still keep doing that, and it allows for projects that have 50+ views, for example, to avoid repeating this sort of code.

As for customizing how the data is fetched, I thought the point of #973 was that it made components more reusable/testable. The way I see it, components in general don't care or have any control over how their props are constructed, they just care about receiving them, no? Same thing here.

The real problem here is what you pointed out in #992

What I don't like about beforeRouteEnter is you have to sometimes use its counterpart (beforeRouteUpdate) and I think most of the time they would both have the same code. Also, no clean way to handle loading actions or errors in a global/generic way, which I very much think is important.

Other frameworks allow for global error handling in this manner. To name a few: Laravel, ASP.NET MVC, EmberJS.

[posva]

Precisely, it's not always a spinner.

As for the components, you may make them handle the case of empty props or not, it's up to you. Honestly, both cases exist

As for the error handling, network errors should be handled by your HTTP lib, like axios

On Sun, 12 Mar 2017, 01:08 Carlos Gonzales, notifications@github.com wrote:

@posva https://github.com/posva:

However, it doesn't allow you to control how the loading/fetching is handled at a view level, you can only handle it at a global level.

How many different ways to show a spinner can there be? To me it looks like boilerplate. Anyways, with the proposed change, you could still keep doing that, and it allows for projects that have 50+ views, for example, to avoid repeating this sort of code.

As for customizing how the data is fetched, I thought the point of #973 https://github.com/vuejs/vue-router/pull/973 was that it made components more reusable/testable. The way I see it, components in general don't care or have any control over how their props are constructed, they just care about receiving them, no? Same thing here.

The real problem here is what you pointed out in #992 https://github.com/vuejs/vue-router/issues/992

What I don't like about beforeRouteEnter is you have to sometimes use its counterpart (beforeRouteUpdate) and I think most of the time they would both have the same code. Also, no clean way to handle loading actions or errors in a global/generic way, which I very much think is important.

Other frameworks allow for global error handling in this manner. To name a few: Laravel https://laravel.com/docs/5.4/errors#the-exception-handler, ASP.NET MVC https://docs.microsoft.com/en-us/aspnet/core/fundamentals/error-handling, EmberJS https://guides.emberjs.com/v2.5.0/routing/loading-and-error-substates/.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vuejs/vue-router/issues/1241#issuecomment-285910697, or mute the thread https://github.com/notifications/unsubscribe-auth/AAoicaXHkN10_3UCOFj-_oFlavAvNgEOks5rkzeWgaJpZM4MaVWy .

[rhyek]

Main thing here is avoiding boilerplate code:

• Most projects that use loading animations won't care to display a different one per view, they will probably configure one once. This will give you consistency and easier maintainability in larger projects.

• You also don't have to check for empty props everywhere in your templates (repetitive, bloated). I think it would be great/appropriate to utilize Vue's functionality of validating props and handle the empty checking there and keep your templates clean. Separation of concerns.

• Using beforeRouteEnter and beforeRouteUpdate means two things:

  • You repeat code (same API requests in both hooks)
  • It is not clear what should happen if your server responds with an error. Currently Vue.config.errorHandler isn't even called if an exception is thrown in beforeRouteEnter. This means things like sentry don't log these errors, potentially. Is this intended?

As for the error handling, network errors should be handled by your HTTP lib, like axios

I think this depends on what scope you intend vue-router to have. Currently, it doesn't do anything network-related, so you would be right in that sense. Also, my example earlier showed:

  errorHandlers: {
    301: (to, from, next, error) {
      next(error.response.headers.Location)
    },
    400: require('components/BadRequest'),
    401: () => {
      store.commit('logout') // store will navigate to home route or whatever
    },
    404: require('components/NotFound')
    // 400 and 404 will cause Vue to render their respective components
    // while optionally having already navigated to the new URL
  }

This probably doesn't make sense since network operations are done by third-party libraries, so Vue wouldn't reliably know when a 404 is thrown, for example, but you could still do something like:

const router = new VueRouter({
  errorHandler(to, from, next, error) {
    // check for status here
  }
})

This would work even nicer if the props function handled like a Promise internally, actually, so vue-router wouldn't need to know what type of error was thrown (network or not) and any uncaught error would eventually be sent to Vue.config.errorHandler in a consistent way (Promise.reject). That way even when just doing the following it would work the same way:

{
  routes: [
    { to: '/', component: Component, props(to) {
      return { id: undefined.id + 1 } // error
    } }
  ],
  errorHandler(to, from, next, error) { // basiscally props().catch()
    // handle
    throw error // Vue.config.errorHandler
  }
}

Edit: Also:

<Spinner v-if="!obj"/>
<div v-else>
{{ obj.property }}
</div>

obj being falsy doesn't necessarily mean your network request isn't yet complete. What do you do if you get a 404, for example?

[posva]

Closing in favour of #1923