Skip to content
This repository has been archived by the owner on Jul 15, 2019. It is now read-only.

Remove baseURI from History getUrl result #96

Closed
wants to merge 1 commit into from

Conversation

Arturszott
Copy link

If baseURI is set, history handling can't match the current Route to the one set in routes config. As a result, browser's url is shortened from:

http://www.example.com/base/location/route

to

http://www.example.com/route

This PR includes removal of the baseUrl part while getting the url from history module.

@Vijar Vijar added the ready label Oct 26, 2015
@lingyan
Copy link
Contributor

lingyan commented Oct 27, 2015

Thanks @Arturszott I will look into this today.

@yahoocla
Copy link

CLA is valid!

@lingyan
Copy link
Contributor

lingyan commented Oct 28, 2015

@Arturszott

Supporting baseURI could be tricky for isomorphic apps. For example, removing base path from getUrl() works for client side, but might break for server side if the server side routing is set up differently with the full path.

After careful consideration, we decided not to support baseURI in fluxible-router to keep the logic simpler. Since handleHistory allows you to configure your own history implementation (https://github.com/yahoo/fluxible-router/blob/master/docs/api/handleHistory.md#options), you can extend our History implementation for your app, and override the getUrl() to fit your need.

Thanks for your contribution!

@lingyan lingyan closed this Oct 28, 2015
@lingyan lingyan removed the ready label Oct 28, 2015
@lo1tuma
Copy link

lo1tuma commented Oct 30, 2015

According to the documentation of handleHistory it is only used on the client, as there is no history-API on the server.

The handleHistory higher-order component handles the browser history state management.

So your argumentation against considering baseURI sounds very strange to me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants