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

[Urgent]Can NOT match 'http://abc.com?trackingid=abc123' #636

Closed
camsong opened this issue Dec 23, 2014 · 17 comments
Closed

[Urgent]Can NOT match 'http://abc.com?trackingid=abc123' #636

camsong opened this issue Dec 23, 2014 · 17 comments

Comments

@camsong
Copy link

camsong commented Dec 23, 2014

I tried to use
<Route name='app' handler={App} path='*' />

* is supposed to match anything, but it can not match the url in title, and it will make a redirection to
http://abc.com#/ and lost the debug parameter. which is quite unfortunate.
I'm deadly need to fix this in my PRODUCTION env, thanks for your help guys.

@camsong camsong changed the title Can NOT match 'http://abc.com?debug=true' [Urgent]Can NOT match 'http://abc.com?trackingid=abc123' Dec 23, 2014
@gaearon
Copy link
Contributor

gaearon commented Dec 23, 2014

If you use hash location (#), http://abc.com/?something=123 is not a valid URL for your website. A valid url would be http://abc.com/#/?something=123.

That being said, I agree that redirect erasing query is unfortunate. For an urgent (but untested) fix, you can change HashLocation.js:

function ensureSlash() {
  var path = getHashPath();

  if (path.charAt(0) === '/')
    return true;

  // ------------------
  // I added these three lines:
  if (window.location.search) {
    path += window.location.search;
  }
  // ------------------

  HashLocation.replace('/' + path);

  return false;
}

@camsong
Copy link
Author

camsong commented Dec 23, 2014

@gaearon however http://abc.com/?something=123#/ will not make redirect and hit the router. it seems the #/ means a lot, is there any ways to add this directly.

@gaearon
Copy link
Contributor

gaearon commented Dec 23, 2014

I don't quite understand why do you use URL like this at all?

It is supposed to be http://abc.com/#/?lala=123, not the other way around. Query always goes at the end.

Finally, to avoid #, you can just use HistoryLocation instead of HashLocation:

https://github.com/rackt/react-router/blob/master/docs/api/run.md#location-optional

@camsong
Copy link
Author

camsong commented Dec 23, 2014

HistoryLocation looks awesome, however we have to stick to HashLocation for now, since the server used a java server which can not work with HistoryLocation right now or the near future.
Also, we have to use the trackingid parameter to let the server to track the user's activity.

Previously, in v0.10, http://abc.com?trackingid=123 will head to http://abc.com?trackingid=abc123#/ directly without make a redirection, which works like a charm.
So it will be pretty cool to make it works the same in v0.11

@gaearon
Copy link
Contributor

gaearon commented Dec 23, 2014

I think I'm starting to understand.

Previous versions of router didn't support query (part after ?), so it wasn't parsed on the client at all. Thus http://abc.com?trackingid=abc123#/ worked.

In 0.11, query is fully supported (you can read it in rotes and change it in transitionTo or replaceWith), so now it goes after /#/ part.

Can you make your server emit http://abc.com/#/?trackingid=abc123 instead of http://abc.com/?trackingid=abc123#/`?

@camsong
Copy link
Author

camsong commented Dec 23, 2014

I mean http://abc.com?trackingid=abc123#/ still works on 0.11, it will match the <Route name='app' handler={App} path='/' />.

However, http://abc.com?trackingid=abc123 will emit
Warning: No route matches path "". Make sure you have <Route path=""> somewhere in your routes
and then redirect to http://abc.com and hashed changed to http://abc.com/# eventually. missed the trackingid finally.

If query is fully supported is true, then http://abc.com?trackingid=abc123 should redirect to http://abc.com/#/?trackingid=abc123 instead of http://abc.com, which is not reasonable.

Maybe the redirection is caused by No route matches path "", so how can I create a route to make it match http://abc.com or http://ab.com?trackingid=123 ?

@gaearon
Copy link
Contributor

gaearon commented Dec 23, 2014

If query is fully supported is true, then http://abc.com?trackingid=abc123 should redirect to http://abc.com/#/?trackingid=abc123 instead of http://abc.com, which is not reasonable.

I fully agree with you here! This is what should happen if you apply small patch I wrote above.

Let's wait for maintainers to weigh on this, but this should be a working solution for now.

@ryanflorence
Copy link
Member

If you're using hash location, I think it should work like this:

  1. Entry url: http://abc.com?trackingid=abc123
  2. Post router setup url: http://abc.com?trackingid=abc123#/
  3. Queries that the router cares about: http://abc.com?trackingid=abc123#/some/path?fake=query

In other words, we should treat a "native" query as just part of the base url that we don't care about. Hash routing, including router query params, come after the hash.

@ryanflorence
Copy link
Member

A full page redirect I think is a bad idea, there are things the server might be doing with that and we'll mess up analytics/logs/etc. with a needless redirect. We can just ignore everything before the hash, and read in our fake query params.

@gaearon
Copy link
Contributor

gaearon commented Dec 23, 2014

This makes sense. Pretty much how it works for path right now.

@ryanflorence
Copy link
Member

I actually thought that's how it worked right now D:

@mjackson
Copy link
Member

@rpflorence That's not how it currently works?

@gaearon
Copy link
Contributor

gaearon commented Dec 23, 2014

Currently pre-hash query is erased. Pre-hash path is left intact.
This is correct in 0.11.

In 0.10, pre-hash query was preserved.

@mjackson
Copy link
Member

Geez. Edge cases..

@gaearon
Copy link
Contributor

gaearon commented Dec 23, 2014

This was introduced in a07003e.

Ah you fixed it already. Great!

@camsong
Copy link
Author

camsong commented Dec 24, 2014

👍 Thanks for the great works!

How about release a new version for this? @mjackson

@saulshanabrook
Copy link

Are query strings still persisted on changing links?

@lock lock bot locked as resolved and limited conversation to collaborators Jan 24, 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

5 participants