Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Universally allow strings or objects for locations #2186

Closed
timdorr opened this issue Oct 7, 2015 · 3 comments
Closed

Universally allow strings or objects for locations #2186

timdorr opened this issue Oct 7, 2015 · 3 comments
Labels

Comments

@timdorr
Copy link
Member

timdorr commented Oct 7, 2015

As mentioned in #2177, I like to propose that any API that accepts a location object also accepts a string form of the location (and vice versa).

The reasoning behind this is a matter of convenience, now that history has deprecated calling createLocation statically and requires a history instance to use the function. A history may not always be readily available, so it requires more boilerplate to get access to that API. In addition, there could be a performance impact from creating and dealing with a history object in cases that don't need it.

Furthermore, the concept of a location object is relatively opaquely defined. It's a POJO with an arbitrary set of fields that may change over time. So, spontaneous creation of location objects by the user are a bad pattern, as they will likely be an upgrade headache at some point in the future and are error-prone. URL strings on the other hand have a very concrete, standardized definition that createLocation will always support. They're the "future-proof" way of handling URLs.

And as some may have noticed, match already does this, so it's not unprecedented. In fact, it's a really nice addition 😄 I believe it's mainly just matchRoutes and useRoutes.match that need to support this, but I'm probably missing some other uses. It could also be applied to the history library, but only if that's a sensible change.

If others think this is a good idea, I can work up a PR to implement the changes. In a perfect world, it won't break any APIs, just augment them, so the impact to upgraders should be low.

@mjackson
Copy link
Member

mjackson commented Oct 7, 2015

I think this is a great idea. We already have the concept of a path, which is the string you're describing here.

I think in addition we should allow a location-like object that we can coerce into a real location using createLocation if we need to. This is the same object I was trying to describe in #2177.

@taion
Copy link
Contributor

taion commented Nov 19, 2015

I think this can be the genesis of a pretty significant API cleanup.

I'd like to push the "strings or objects" concept one level higher into history.

Ideally, history.push and history.replace should support either strings or objects, and we would move to those as the preferred APIs over pushState and replaceState.

We could then modify <Link> to accept either a string or an object for props.to, and forward that directly to history.push (and something analogous for active state).

This could then allow us to just set up named routes as a history enhancer, e.g.:

const history = useNamedRoutes(createHistory({routes}))

This could work by building the route map, then wrapping the underlying push, replace, and isActive accordingly.

@timdorr
Copy link
Member Author

timdorr commented Dec 27, 2015

This is done 😄

@timdorr timdorr closed this as completed Dec 27, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants