faceyspacey / redux-first-router

🎖 seamless redux-first routing -- just dispatch actions
MIT License
1.56k stars 143 forks source link

Optional path parameters? #83

Closed pelotom closed 7 years ago

pelotom commented 7 years ago

In redux-little-router one can specify optional parameters in route paths, e.g. /some/path/(:foo), and then both /some/path and /some/path/hello would map to the same route, in the former case passing { foo: undefined } as the parameters. Is there a similar mechanism in redux-first-router whereby those could both map to the same action but with an optional field in the payload?

faceyspacey commented 7 years ago

Yea, I'm gonna optimize support for various regex-like stuff you can do. Some stuff works now, it's just "undefined behavior." What you have likely won't match, but you should be able to extract unnamed parameters (*) into payload['0'] for example.

Experiment and let me know what you find. I want to revisit this asap.

gouegd commented 7 years ago

I've been thinking about this one. Adding several paths for the same route is not currently possible because of how JS works.

// no good
{
  PROFILES: '/profiles/:id/:section',
  PROFILES: '/profiles/:id',
  PROFILES: '/profiles'
}

Following another issue (https://github.com/faceyspacey/redux-first-router/issues/94) I'm thinking about supporting arrays of paths (or arrays of objects) for each route, like this:

/// with array support
{
  PROFILES: ['/profiles/:id/:section', '/profiles/:id', '/profiles']
}

An array support would be helpful for other use cases too (see other issue for instance), and it should be simple to get from optional path params like /profiles/(:id)/(:section) to the array above. So, users would input optional params and we could turn it into an array of paths. Optionally the users can also just input a array of paths directly.

What do you guys think ?

Currently for optional path params I'm duplicating entries in the routeMap, so I need to resort to some tricks to avoid having super-repetitive condition checks everywhere.

faceyspacey commented 7 years ago

Optional params is on the roadmap. Feel free to implement it if you need it sooner.

That said the only time this is really needed is when the paths are user generated like file and folders on GitHub. So imagine one optional param that can represent any number of path segments and will give you its string in a param. Ie this /(*).

I bring that example because that is something we CANNOT currently accomplish via other ways. However it is a rare need.

As for more basic optional params mentioned here, I personally prefer lots of types in my reducers rather than having a single type and comparing payload params. It's just so easy to flatly switch over types. I don't care about the repetition.

FYI all we are solving by consolidating the PROFILES into one route is less cases to switch over in reducers. And perhaps not having to assign the same thunk 3 times. Component code shouldn't have any repetition since u write a reducer once that derives reusable state into a clean single variable you can connect to your components.

So my opinion is that reducer action type repetition is a good thing.

In the future u may appreciate the difference, and revert to breaking it back up to 3 routes. At which point it's a feature. As apps grow and become more custom, u will inevitably add more features to differentiate between the 3 routes.

So basically we are taking about a convenient option here for people that unnecessarily cringe at too many cases in their reducer. A feel nice thing, especially in the beginning stages of your app where u have high hopes for DRY and no repetition lol. I make that sound bad but ultimately it's a fine use case since accessibility is key to marketing this package and that's a common instinct to have. That's why we will support optional params. I'm not sure about the manually provided array, but I'm not staunchly opposed. At the end of the day, this would be a great first PR. Reg-to-exp already works with optional params. I just don't do anything with them. U should take a look into the pathToAction.js and actionToPath.js files.

silouanwright commented 7 years ago

I had to think about what you just said @faceyspacey because I didn't get it. I think I sort of get it now and wanted to flesh it out for everyone, hopefully I get this right.

The key here is the page.js component.

So here you have your page.js component which you use in your root react component to show different components based on the route (this essentially replaces the <Route /> stuff):

// reducers -> page.js
const components = {
    HOME: 'Home',
    PRODUCTS: 'Products',
    [NOT_FOUND]: 'NotFound',
};

export default (state = 'HOME', action = {}) =>
    components[action.type] || state;

So what he's basically getting at with the multiple types stuff is this:

// routesMap.js

    PRODUCTS: {
        path: '/products/:productID/',
        thunk: (dispatch => {
            // async code here
        }),
    },
    PRODUCTS_WITH_AFFILIATE: {
        path: '/products/:productID/:affiliateID',
        thunk: (dispatch => {
            // async code here
        }),
    },

The key here, is the page.js component. All you need to do is update it with your new type:

// reducers -> page.js
const components = {
    HOME: 'Home',
    PRODUCTS: 'Products',
    PRODUCTS_WITH_AFFILIATE: 'Products',
    [NOT_FOUND]: 'NotFound',
};

export default (state = 'HOME', action = {}) =>
    components[action.type] || state;

Notice how they both point to "Products". Now you just use page.js as your source of truth.

That's the beauty of the page.js component. You have types mapped to routes, but you have multiple types mapped to pages inside this page.js reducer. So you use this page reducer in this way:

function App({ page }) {
    const pages = {
       Products: <Products />,
    };

    return (
        <div>
            {pages[page]}
        </div>
     )
}

function mapStateToProps(state) {
    return {
        page: state.page,
    };
}

And you're done. Ideally you use whatever difference in the payload inside the components dynamically, e.g.

<div>
{props.products.affiliate && (
   <h1>Our affiliate is {props.products.affiliate}</h1>
)}

Or maybe faceyspacey is suggesting this?

{props.types.PRODUCT_AFFILIATE && (
   <h1>Our affiliate is {props.products.affiliate}</h1>
)}
</div>

I'm actually not totally against it... I'll need to use it and see what type of ramifications this has. The main thing here is if I consider PRODUCT & PRODUCT_AFFILIATE the same page, I literally cannot use types in conditionals... I MUST defer to page.js (unless I feel like doing `if PRODUCT && PRODUCT_AFFILIATE && PRODUCT_SOMETHING_ELSE every time I want to target a page, which I'm just not going to do). Does that increase complexity? When I'm looking at types inside the redux console, would I rather see one consistent type per route, or multiple types based on the kind of route.. I can't really tell.

That being said, I'm wiring it up right now and if it feels decent, it's going into production.

pelotom commented 7 years ago

It's certainly doable by having a different action type per permutation of absent vs. present parameters, but it feels not as nice to use as it could be.

silouanwright commented 7 years ago

@pelotom I think what you're referring to is how this violates DRY but as far as I know, @faceyspacey isn't really staunch on the DRY stuff. Maybe the flexibility and having more types makes customization and handling conditional state showing easier in the long run? I think that's the big question here... are we nitpicking over what will amount to not the greatest amount of duplication... maybe having more types speaks better to the state our program is in, rather than having to go to the component and having to see which pieces of data are specifically rendering these views (we also don't have any named associations to these different views). It's not like we're discard the extra data at the end of the path... it's most likely going to be used somehow. That being said, I'm playing devils advocate here. The approach is starting to make sense but I'd need to defer to people who try both approaches in larger applications and then make my opinion from there.

pelotom commented 7 years ago

@reywright what I'm advocating doesn't prohibit using as many action types as you like, so it's no less flexible. It just allows avoiding a combinatorial explosion of different action types in certain cases. For example, if what you have is conceptually MY_SCREEN: 'foo/(:foo)/bar/(:bar)/baz/(:baz)', do you want to have to write these routes?

MY_SCREEN__FOO_BAR_BAZ: 'foo/:foo/bar/:bar/baz/:baz',
MY_SCREEN__FOO_BAR_NOBAZ: 'foo/:foo/bar/:bar/baz',
MY_SCREEN__FOO_NOBAR_BAZ: 'foo/:foo/bar/baz/:baz',
MY_SCREEN__FOO_NOBAR_NOBAZ: 'foo/:foo/bar/baz',
MY_SCREEN__NOFOO_BAR_BAZ: 'foo/bar/:bar/baz/:baz',
MY_SCREEN__NOFOO_BAR_NOBAZ: 'foo/bar/:bar/baz',
MY_SCREEN__NOFOO_NOBAR_BAZ: 'foo/bar/baz/:baz',
MY_SCREEN__NOFOO_NOBAR_NOBAZ: 'foo/bar/baz',

and then elsewhere all the corresponding component maps needed to consolidate them?

faceyspacey commented 7 years ago

we are gonna have this fellas. it does make sense as a user's decision. and keep in mind, i'm not personally that amped about it. so that just means it makes sense, and we gotta have it.

help on code appreciated.

gouegd commented 7 years ago

At the end of the day, this would be a great first PR

@faceyspacey Do you refer to providing arrays of paths, optional params, or both ?

I appreciate your point about repetition in reducers not being a bad solution. I'm also finding myself repeating this kind of branches when creating an action (pointing to different types whether there's a payload or not, which depends on the current location). I use custom actions to have a better dev experience on this.

Also as a side note I've not actually been using reducers so far, instead relying entirely on custom information passed in the routeMap per route. This is an alternative that works, but that ends up creating lots of repetitions compared to reducers, in certain cases. So I'm now moving to using reducers more.

faceyspacey commented 7 years ago

Optional params. Keep the API surface smaller.

faceyspacey commented 7 years ago

Ok, this is done. All matching capabilities of the path-to-regexp package we use are now supported, except unnamed parameters.

Let's go through what we now support. Usually it's best to start with the simplest example, but I think most people looking at this get this stuff. We'll start with one of the more complicated use cases just to see how far we can take this:

Multi Segment Parameters

So for example, imagine you're github.com and you're now using Redux-First Router :), and you need to match all the potential dynamic paths for files in repos, here's how you do it:

const routesMap = {
   REPO: '/:user/:repo/block/:branch/:filePath+'
}

So that will match:

but not:

And if you visit that URL, you will see it in fact doesn't exist. If it did, you would use an asterisk (for 0 or more matches as in regexes) like so:

const routesMap = {
   REPO: '/:user/:repo/block/:branch/:filePath*'
}

So the above 2 options will match a varying number of path segments. Here's what a corresponding action might look like:

const action = {
   type: 'REPO',
   payload: {
       user: 'faceyspacey',
       repo: 'redux-first-router',
       branch: 'master',
       filePath: 'src/pure-utils/pathToAction.js'
   }
}

Pretty cool, eh!

The inspiration actually came from a PR I did to CodeSandBox. I didn't actually implement this there, but I was thinking about it around that time. I.e. that CodeSandBox should have the full file paths in the URLs like github. Currently it's as a URL-encoded query param, but in the future (perhaps with RFR), they'll be able to do what Github does as well.

Optional Single Segment Parameters

However, you usually just want to add optional single segment params like this:

const routesMap = {
   ISSUES: '/:user/:repo/issues/:id?'
}

So with that you can visit both:

Here's the 2 actions that would match that respectively:

And that's basically the "80% use-case" most powerful feature here. I.e. when you want to use the same type for a few similar URLs, while getting an optional parameter.

note: you can also have optional params in the middle, eg: /foo/:optional?/bar/:anotherOptional? So all 3 of /foo/bla/bar/baz and /foo/bar/baz and /foo/bar will match :)

Optional Static Segments ("aliases")

The absolute most common (and simpler) use case for such a thing is when you want /items and /items/list to have the same type (because they are aliases of each other ). You accomplish it slightly differently:

const routesMap = {
  HOME: '/home/(list)?',
}

or

const routesMap = {
  HOME: '/home/(list)*',
}

Both are the same. It would be nice if you didn't have to wrap list in parentheses, but you have to. That's fine. Keep in mind this isn't a parameter; the 2nd path segment has to be list or not be there.

Also, the common convention we should use is the the question mark ? instead of the asterisk *. We should reserve the asterisk for where it has unique capabilities, specifically the ability to match a varying number of path segments (along with +) as in the initial examples with the github file paths.

Regexes

Another thing to note about the last one is that (list) is in fact a regex. So you can use regexes to further refine what paths match. You can do so with parameter segments as well:

const routesMap = {
  HOME: '/posts/:id(\\d+)',
}

So essentially you follow a dynamic segment with a regex in parentheses and the param will have to match the regex. So this would match:

Multiple Multi Segment Parameters

Last use case: say you want to have multiple params with a varying number of segments. Here's how you do it:

const routesMap = {
  HOME: '/home/:segments1+/bla/:segments2+',
}

So a url like /home/foo/bar/bla/baz/bat/cat will result in an action like this:

const action = {
   type: 'HOME',
   payload: {
       segments1: 'foo/bar',
       segments2: 'baz/bat/cat'
   }
}

So yes, you can have multiple "multi segments parameters".

One thing to note is you could also accomplish this like this: HOME: '/home/:segments1(.*)/bla/:segments2(.*)'.


Final reminder: if you do plan to use multi segment parameters, they have to be named. This won't work: /home/(.*)/bla/(.*).

Well, the truth is it will, and given the previous URL you will have a payload of:

const action = {
   type: 'HOME',
   payload: {
       0: 'foo/bar',
       1: 'baz/bat/cat'
   }
}

But consider that "undefined" functionality. Don't rely on that. Name your segments! Like any other key in your payload. That's the goal here. Originally I had the idea of making an array at payload.segments, but then I realized it was possible to name them. So naming all params is the RFR way.


Lastly, to get this do:

yarn upgrade redux-first-router@next
pelotom commented 7 years ago

Looks awesome, can't wait to try it!

gouegd commented 7 years ago

Great samples, glad you already found some time for this !

ygamretuta commented 7 years ago

how do I solve the 1.x dependency with rudy-match-path?

faceyspacey commented 7 years ago

@ygamretuta can you elaborate??

ygamretuta commented 7 years ago

screenshot from 2017-09-08 13-14-52

faceyspacey commented 7 years ago

ok, i'll update the peer deps. im just gonna remove em all. they arent needed. ..but that should still be working. that didnt stop the packaging from completing and the app running for me.

either way, i'll have it updated within the hour.

faceyspacey commented 7 years ago

https://www.npmjs.com/package/rudy-match-path https://www.npmjs.com/package/redux-first-router-link

both updated

gmattar commented 6 years ago

Hi, since routeMap is not a list, how can I have precedence for routes?

I want to create some first level static routes like this: /about /signin /logout

and then routes with parameters to match custom names rule: /:store

examples /my_own_store /buy_my_books

I'm trying to avoid/store/:storename. is it possible?

Thanks

faceyspacey commented 6 years ago

The order of "elements" in an object are respected in all browsers (old and new) despite that the spec doesn't require this. There's another issue somewhere where this is discussed.

So in short, just treat it like an array and you can trust the order precedence.

BrendanFDMoore commented 6 years ago

Is there any way to force a path parameter to be interpreted as a string, even if it contains only numeric digits? We have a case of a zero-padded number that is being trimmed to just the numeric value that we'd prefer to keep the leading attached to. I can't seem to figure out how to do this from the docs.

BrendanFDMoore commented 6 years ago

Given the example of:

const routesMap = {
   LISTING: '/listing/:id'
}

and a visit to www.example.com/listing/0012345, I get a route action with a payload like this:

const action = { type: 'LISTING', payload: { id: 12345} }

but I want to make it like this:

const action = { type: 'LISTING', payload: { id: '0012345'} }

Is that possible?

faceyspacey commented 6 years ago

Try toPath and fromPath. Otherwise the next version will allow lower level customization.

On Wed, Jul 11, 2018 at 1:43 PM Brendan Moore notifications@github.com wrote:

Given the example of:

const routesMap = { LISTING: '/listing/:id' }

and a visit to www.example.com/listing/0012345, I get a route action with a payload like this:

const action = { type: 'LISTING', payload: { id: 12345} }

but I want to make it like this:

const action = { type: 'LISTING', payload: { id: '0012345'} }

Is that possible?

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/faceyspacey/redux-first-router/issues/83#issuecomment-404303012, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJcbNCTpHTC8iIzFlQXi9OyDimXg2e-ks5uFmNUgaJpZM4O5xPb .

-- James from http://faceyspacey.com - "The Startup Incubator" http://facebook.com/faceyspacey http://twitter.com/FaceySpacey http://www.linkedin.com/in/faceyspacey