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

"Warning: Automatically setting basename using <base href> is deprecated" errors #3387

Closed
tomduncalf opened this issue Apr 26, 2016 · 30 comments

Comments

@tomduncalf
Copy link

tomduncalf commented Apr 26, 2016

Version

2.0.0

Test Case

https://github.com/tomduncalf/react-router-base-issue (issue is not reproducable in JSBin, attempted at http://jsbin.com/poqige/edit?html,js,output, so I suspect the issue is related to ES6/Babel/webpack module bundling/loading somehow)

Steps to reproduce

  1. Add a <base href="http://whatever"> tag to the head of a page containing react-router's HTML using the default browserHistory
  2. Load the page - history.js will output 2 errors, Warning: Automatically setting basename using <base href> is deprecated and will be removed in the next major release. The semantics of <base href> are subtly different from basename. Please pass the basename explicitly in the options to createHistory
  3. Change the react-router history to use history where history is const history = useRouterHistory(createHistory)({ basename: 'http://whatever' }), which should fix the problem (as basename is now explicitly passed)
  4. Reload the page

Expected Behavior

Console errors do not appear

Actual Behavior

2 console errors still appear

Additional notes

I have done a bit of digging into this, and I believe what is going on is something like:

(the same is also true of https://github.com/reactjs/react-router/blob/master/modules/browserHistory.js)

So because this export is actually an import then an export, the import triggers a call to createRouterHistory for the default export of hashHistory, which in turn creates a new history without the options we need to pass to it to avoid an error.

A few options I can think of are:

  1. Ignore the errors, they only show in dev (not ideal)
  2. Fix the issue on the history module end somehow, e.g. by passing a flag to silence these errors
  3. Fix the issue in react-router, e.g. make hashHistory export a function that calls createRouterHistory rather than calling it directly, or to somehow pass the options through

Would be great to get your input on this, first time I have looked in the react-router codebase otherwise I might have dug deeper and tried to fix :)

@taion
Copy link
Contributor

taion commented Apr 26, 2016

Please follow the guidelines on https://github.com/reactjs/react-router/blob/master/docs/guides/MinimizingBundleSize.md#import-from-react-routerlib.

I'm thinking about lazily initializing the history objects by using a membrane, but that's some ways away.

@taion
Copy link
Contributor

taion commented Apr 26, 2016

Should be fixed by remix-run/history#283 (or maybe #3388 here) at some point, though.

@tomduncalf
Copy link
Author

Thanks @taion - this presents a good excuse to try and get Webpack 2 tree-shaking working, and if not then it is a good to know it should be fixed at some point.

@tomduncalf
Copy link
Author

I didn't have any luck with the tree-shaking route, unless I've missed something - although I've confirmed Webpack is loading the jsnext:main module by deleting all directories except es6 from react-router in node_modules, it is still hitting the index.js in there, which is still importing e.g. browserHistory and throwing the error

@taion
Copy link
Contributor

taion commented Apr 27, 2016

It's quite possible we were too optimistic initially about how tree-shaking semantics work out. Let's see if we can get a history 2.1.1 with a patch released that doesn't warn until the initial listen.

@tomduncalf
Copy link
Author

Yeah, I think that might be what is going on. For now I am using the direct
imports and that has fixed it, so thanks for the help!

On 27 April 2016 at 12:50, Jimmy Jia notifications@github.com wrote:

It's quite possible we were too optimistic initially about how
tree-shaking semantics work out. Let's see if we can get a history 2.1.1
with a patch released that doesn't warn until the initial listen.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#3387 (comment)

@ochowie
Copy link

ochowie commented Apr 27, 2016

So with this new solution, is the approach to use const history = useRouterHistory(createHistory)({ basename: 'http://whatever' }) as tomduncalf mentioned? Looking at the changed code it seems like it will still issue the warning if only the tag is present. Will there be a change to enable browserHistory to accept options as well?

@tomduncalf
Copy link
Author

@ochowie, yes, for now this is the solution (and make sure you only directly import the modules you need from react-router). Would be nice if browserHistory accepted options I agree!

On 27 Apr 2016, at 20:13, ochowie notifications@github.com wrote:

So with this new solution, is the approach to use const history = useRouterHistory(createHistory)({ basename: 'http://whatever' }) as tomduncalf mentioned? Looking at the changed code it seems like it will still issue the warning if only the tag is present. Will there be a change to enable browserHistory to accept options as well?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

@taion
Copy link
Contributor

taion commented Apr 27, 2016

With history 2.1.1, the warning should no longer be present if you do

import { browserHistory } from 'react-router'

For minimum bundle size you shouldn't do this.

And for the time being, you will need to use the custom history code path if you need a basename.

@ochowie
Copy link

ochowie commented Apr 27, 2016

@taion I have history 2.1.1 and react-router 2.3.0 and I'm getting that warning when I do
import { browserHistory } from 'react-router'

@taion
Copy link
Contributor

taion commented Apr 28, 2016

The feature is actually deprecated. You need to create a custom history if you want to use basename.

@ochowie
Copy link

ochowie commented Apr 28, 2016

I'm using the tag because I'm serving from Nginx and from what I've
read it's required. By deprecating this functionality is the message that
serving from node the recommended approach?
On Wed, Apr 27, 2016 at 8:29 PM Jimmy Jia notifications@github.com wrote:

The feature is actually deprecated. You need to create a custom history if
you want to use basename.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#3387 (comment)

@taion
Copy link
Contributor

taion commented Apr 28, 2016

I'm not sure what you mean. If you need basename support, create your own history with something like

useRouterHistory(createBrowserHistory)({ basename })

Otherwise you'll have to elaborate on what you're trying to do.

@ochowie
Copy link

ochowie commented Apr 28, 2016

So at this point I think it's more of a documentation question. In the
current docs the examples use browserHistory to create the history. The
issue is this doesn't work with Nginx which seems to require a base tag. It
seems like the examples specifically target serving from node. In the past
this was transparent since history used those tags but it might be worth a
comment or an example to address this issue.
On Wed, Apr 27, 2016 at 8:54 PM Jimmy Jia notifications@github.com wrote:

I'm not sure what you mean. If you need basename support, create your own
history with something like

useRouterHistory(createBrowserHistory)({ basename })

Otherwise you'll have to elaborate on what you're trying to do.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#3387 (comment)

@taion
Copy link
Contributor

taion commented Apr 28, 2016

I don't understand why Nginx requires a base tag. The reason we deprecated this feature was because the semantics on <base href> don't match the ones on basename.

@ochowie
Copy link

ochowie commented Apr 28, 2016

Seems like this wasn't necessarily a react-router or nginx issue. I was using a relative path for my bundled script rather than an absolute path and that was why I would get an error if I wasn't using the base tag. Since I can use absolute paths right now I'm good, but that might not be true or desired in all cases. Sorry if this wasted your time.

@ghost
Copy link

ghost commented Jun 21, 2016

Why was this issue closed? The consensus seemed to be that

import { browserHistory } from "react-router";

would indeed prevent the warning, but also that this is obviously just a workaround, as it leads to problems that are far worse than a mere warning in dev mode (increased bundle size, unused variables, thus linting issues, …).

Would you agree to re-open this issue until the problem is fixed, be it just to keep track of it?

@taion
Copy link
Contributor

taion commented Jun 21, 2016

That was not the consensus. The fix was related to additionally removing the warning if using that form without a <base> tag. There never was an issue if you got the browser history some other way.

However, using <base href> to set basename remains deprecated. You need to use the custom history path if you need a basename.

@ghost
Copy link

ghost commented Jun 21, 2016

I do provide a custom history in my application, but my team still needs <base href> for other reasons; so, we don't rely on it to set up the history.

There never was an issue if you got the browser history some other way.

Wait; wasn't the issue that @tomduncalf described exactly that the warning still persists even if a custom history is created? Because that's what I'm still observing in the current stable.

@taion
Copy link
Contributor

taion commented Jun 21, 2016

No, that's a separate issue.

If you're using <base>, for now just explicitly set basename to ''.

@ghost
Copy link

ghost commented Jun 21, 2016

Thanks, but I'm not sure if this does what I want; in our scenario, / should point to http://example.com/some-path/, so wouldn't I need to set basename to /some-path/ instead of '' (which I assume to be equivalent to '/')?

@taion
Copy link
Contributor

taion commented Jun 21, 2016

Then you're using a basename anyway, and you should set it in your custom history.

@ghost
Copy link

ghost commented Jun 21, 2016

I'm getting the feeling we're talking at cross-purposes. 😄
That's exactly what I did, as I pointed out. The issue is really that the warning is still there.

@taion
Copy link
Contributor

taion commented Jun 21, 2016

Check that you're importing and setting up your history appropriately.

Otherwise, reinstall and make sure you're using the correct version of history.

@chrisg220
Copy link

@taion After I add the following code: const history = useRouterHistory(createHistory)({ basename: 'http://whatever' })

I get the following error: "createHistory is not a function"

I tried importing "createHistory from react-router, but that didn't work. Do you know how I should move forward?

@honpery
Copy link

honpery commented Aug 27, 2016

@iyobo
Copy link

iyobo commented Oct 2, 2016

createHistory is no longer a function from 'history'. (v4)
Could someone perhaps give a more updated solution to this?

Also what versions of both the router and hist

@iyobo
Copy link

iyobo commented Oct 2, 2016

Downgraded history to v2.1.2 and all seems to be working fine. No more annoying error.

@nitish24p
Copy link

I'm still getting the above issue.. can someone tell me if theyve got a solution here.. Tried the ones above but to no avail.

@iyobo
Copy link

iyobo commented Oct 22, 2016

@nitish24p

First rm-rf node_modules.
Then make sure your history version is 2.1.2 in package.json.
Then npm install all over again.
Confirm version of history module in node_modules is, in fact, version 2.1.2 (By looking at it's package.json)

Then createHistory will be a function.

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

No branches or pull requests

7 participants