New feature: Passing props to components from vue-router #973

bfanger · 2016-12-02T11:14:52Z

Decouple component from the router by using props:

Documentation: https://github.com/bfanger/vue-router/blob/dev/docs/en/essentials/passing-props.md
Examples: https://github.com/bfanger/vue-router/tree/dev/examples/route-props

Makes it easier to upgrade to those packages.

fnlctrl · 2016-12-02T12:55:06Z

Interesting idea!
/ping @posva @LinusBorg

fnlctrl · 2016-12-02T12:56:25Z

src/util/props.js

+}
+
+function hasProp (component, prop) {
+  if (!component.props) {


Can probably skip this if (!component.props) check since it's already done in https://github.com/bfanger/vue-router/blob/5ad9d480eeed6700a26597484d0c0454d30a13e3/src/util/props.js#L20-L22
Or maybe just inline this function and use the typof expression..

Agreed, i've inlined the hasProp function

fnlctrl · 2016-12-02T12:57:31Z

src/util/props.js

+          const propType = component.props[prop].type
+          if (propType === null || propType === String) {
+            props[prop] = value
+          } else if (propType === Number && value.match(/^-?([0-9]+|[0-9]*\.[0-9]+)$/)) {


Should probably cache this regex literal /^-?([0-9]+|[0-9]*\.[0-9]+)$/

Agreed, done

fnlctrl · 2016-12-02T13:00:45Z

src/components/view.js

@@ -49,6 +51,7 @@ export default {
          matched.instances[name] = undefined
        }
      }
+      data.props = resolveProps(route, component, matched.props && matched.props[name])


~~Should merge with the original data.props, or it would be a breaking change as the current code disallows explicitly passing props to <router-view>, like <router-view foo="bar"/>~~

Edit: Sorry, I was wrong about that..

The props of <router-view> are still passed to the rendered element. The <div> in Hello still gets the class="view"

Did some further testing and setting properties on <router-view> overwrites the values set in data.props, so they are merged somewhere

Yep, that's normal Vue behaviour for any component.

fnlctrl · 2016-12-02T13:06:12Z

src/util/props.js

+      for (const prop in route.params) {
+        const value = route.params[prop]
+        if (hasProp(component, prop)) {
+          const propType = component.props[prop].type


I'm not sure about reading prop types at runtime and converting them to Number if possible.. Maybe we should just leave them as string since url params are string by nature. And if Number was supported, should we also support Boolean?

Also, if this type conversion were to be supported, type can be an array, so we'll have to loop over that should it be the case.

Not ideal, but the alternative was either not to support Number type, or pass it as an string and get a propType warnings and subtle bugs in itemsById.find(item => item.id === id)

I think routing to ID is a common use-case to allow it in the {props: true} scenario

Maybe in that case, the user should provide a function and convert types by himself. This way we avoid the implicit behavior.

I've simplified the behavior of the {props: true} which now sets the props to route.params.
Vue complaining about type is much better than silently ignoring some properties and leaving users confused why it doesn't work.

posva · 2016-12-03T15:48:30Z

I love the idea!
Haven't read all the code yet

LinusBorg · 2016-12-27T10:29:54Z

I think this is a really great idea.

/ping @yyx990803 you should check this out, seems very useful!

yyx990803 · 2016-12-27T17:28:14Z

Looks good! Will review in depth after new year and likely merge when we work on 2.2 for the router.

posva · 2017-01-15T19:06:18Z

test/e2e/nightwatch.config.js

@@ -1,4 +1,7 @@
 // http://nightwatchjs.org/guide#settings-file
+var seleniumServer = require('selenium-server');
+var chromedriver = require('chromedriver');


This line is not necessary

lines are removed

posva · 2017-01-15T19:06:50Z

test/e2e/nightwatch.config.js

@@ -7,7 +10,7 @@ module.exports = {

  'selenium': {
    'start_process': true,
-    'server_path': 'node_modules/selenium-server/lib/runner/selenium-server-standalone-2.53.1.jar',
+    'server_path': seleniumServer.path,


To be consistent we should do server_path': require('selenium-server').path instead

posva · 2017-01-15T19:07:35Z

docs/en/essentials/passing-props.md

+
+### Object mode
+
+When props is an object, this will be set as the component props as-is.


It'd be even better to say when it's useful to pass an object as the props

I've improved the description and code snippet.

yyx990803 · 2017-01-19T17:40:33Z

@bfanger apologies for taking so long to merge this - great work!

baoti · 2017-02-08T06:16:02Z

docs/en/api/options.md

@@ -13,6 +13,7 @@
    name?: string; // for named routes
    components?: { [name: string]: Component }; // for named views
    redirect?: string | Location | Function;
+    props?: boolean | string | Function;


It is Boolean | Object | Function in passing-props.md

bfanger added 2 commits December 2, 2016 10:58

Let the selenium-server and chromedriver packages determine the paths

be7a138

Makes it easier to upgrade to those packages.

Feature: routeConfig.props - Passing props to a component

5ad9d48

fnlctrl reviewed Dec 2, 2016

View reviewed changes

bfanger added 3 commits December 2, 2016 14:49

inlined hasProp + cached the regex literal

0902926

Simplied the { props:true } behaviour which is now very predicable.

608f6e3

Updated docs

5ff882e

LinusBorg added feature 2.x labels Dec 12, 2016

Merge branch 'dev' into dev

a7cd827

posva requested changes Jan 15, 2017

View reviewed changes

Updated docs + re-enabled props functionality

0ac46ca

posva approved these changes Jan 15, 2017

View reviewed changes

yyx990803 merged commit 2288d83 into vuejs:dev Jan 19, 2017

nirazul mentioned this pull request Jan 29, 2017

2.2.0 Changelog error #1125

Closed

ghost mentioned this pull request Jan 31, 2017

[FR] - Pass a promise to new props function #1126

Closed

baoti reviewed Feb 8, 2017

View reviewed changes

rhyek mentioned this pull request Mar 11, 2017

Props function in route definition should handle returning promises #1241

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New feature: Passing props to components from vue-router #973

New feature: Passing props to components from vue-router #973

bfanger commented Dec 2, 2016

fnlctrl commented Dec 2, 2016

fnlctrl Dec 2, 2016 •

edited

Loading

bfanger Dec 2, 2016

fnlctrl Dec 2, 2016

bfanger Dec 2, 2016

fnlctrl Dec 2, 2016 •

edited

Loading

bfanger Dec 2, 2016 •

edited

Loading

bfanger Dec 2, 2016

LinusBorg Dec 2, 2016

fnlctrl Dec 2, 2016 •

edited

Loading

bfanger Dec 2, 2016

fnlctrl Dec 2, 2016

bfanger Dec 2, 2016 •

edited

Loading

posva commented Dec 3, 2016

LinusBorg commented Dec 27, 2016

yyx990803 commented Dec 27, 2016

posva Jan 15, 2017

bfanger Jan 15, 2017

posva Jan 15, 2017

bfanger Jan 15, 2017

posva Jan 15, 2017

bfanger Jan 15, 2017

yyx990803 commented Jan 19, 2017

baoti Feb 8, 2017


		### Object mode

		When props is an object, this will be set as the component props as-is.

New feature: Passing props to components from vue-router #973

New feature: Passing props to components from vue-router #973

Conversation

bfanger commented Dec 2, 2016

fnlctrl commented Dec 2, 2016

fnlctrl Dec 2, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fnlctrl Dec 2, 2016 • edited Loading

Choose a reason for hiding this comment

bfanger Dec 2, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fnlctrl Dec 2, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bfanger Dec 2, 2016 • edited Loading

Choose a reason for hiding this comment

posva commented Dec 3, 2016

LinusBorg commented Dec 27, 2016

yyx990803 commented Dec 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yyx990803 commented Jan 19, 2017

Choose a reason for hiding this comment

fnlctrl Dec 2, 2016 •

edited

Loading

fnlctrl Dec 2, 2016 •

edited

Loading

bfanger Dec 2, 2016 •

edited

Loading

fnlctrl Dec 2, 2016 •

edited

Loading

bfanger Dec 2, 2016 •

edited

Loading