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

Router to provide property for document.referrer #1181

Open
aronslee opened this issue Sep 10, 2013 · 18 comments
Open

Router to provide property for document.referrer #1181

aronslee opened this issue Sep 10, 2013 · 18 comments

Comments

@aronslee
Copy link

For single-page app experiences the document.referrer is not properly maintained.

Router instances should store, update, and provide accessibility to a referrer property.

@ericf

@ghost ghost assigned ericf Sep 10, 2013
@ericf
Copy link
Member

ericf commented Sep 11, 2013

There are some semantics here that need to be worked through…

It makes sense to me to add a req.referrer property, the request object feels like the right place to put this information. When there is only one router instance on the page, and if it's the only changing the URL on the page the logic could work like this:

  1. Person goes to http://example.com/, and a Y.Router instance is created.
  2. router.save('/foo') updates the URL, which causes the router to dispatch with a req.referrer === '/'.
  3. Person hits back button, the URL updates to '/', which causes the router to dispatch with a req.referrer === '/foo'.

This could be implemented by having each router instance maintain state for what the previous URL is, and in fact it already does so today to disambiguate the popstate event. Every time the URL changes, every router instance's _afterHistoryChange() event handler is called, even if that router doesn't have a matching route for the current URL. The routers can store it so when they dispatch they can put that URL as the req.referrer.

The thing to work out here is what this means when there are two routers, I'll keep thinking through that to see if this approach breaks down in those scenarios.

@ericf
Copy link
Member

ericf commented Sep 17, 2013

There are two issue with the semantics around referrer and multiple Y.Router instances. When there are multiple router instances on the page created a different times, what should their referrer be? Consider the following:

  1. Person does a full page load from: http://example.com/ to http://example.com/app/.
    The document.referrer is "http://example.com/"
  2. Create routerOne and call dispatch():
    The req.referrer is "http://example.com/", the same as document.referrer.
  3. Call routerOne.save('/app/foo'):
    The req.referrer is "http://example.com/", the same as document.referrer.
  4. Call routerOne.save('/app/bar'):
    The req.referrer is "http://example.com/app/foo".
  5. Create routerTwo and call dispatch():
    What should req.referrer be? Should it be: "http://example.com/" or "http://example.com/app/foo"?

Should all router instances start with document.referrer as their req.referrer value until they transition to a new URL, in which case the previous URL will be used as the req.referrer? Or should there be globally shared state for what the current referrer is?

I'm inclined to go with the former, and when a router instance is created it starts with the value of document.referrer until it starts building up its own state. This would be same behavior if you used some other library to update the URL before Y.Router was ever added to the page.

@rgrove
Copy link
Contributor

rgrove commented Sep 17, 2013

I agree; starting all routers with document.referrer seems best.

@ericf
Copy link
Member

ericf commented Sep 17, 2013

When I went thought this last night, I actually ended up using shared state between router instances in the same YUI instance; here's the diff: https://gist.github.com/ericf/6594826

@ericf
Copy link
Member

ericf commented Sep 17, 2013

Also, here's the WIP branch ericf/yui3@yui:dev-3.x...router-req

@rgrove
Copy link
Contributor

rgrove commented Sep 17, 2013

When I went thought this last night, I actually ended up using shared state between router instances in the same YUI instance

Why?

@ericf
Copy link
Member

ericf commented Sep 17, 2013

When thinking about it more I was looking at the following case:

var routerOne = new Y.Router(),
    routerTwo = new Y.Router();

function logReferrer(req) {
    Y.log(req.referrer);
}

rotuerOne.route('/foo', logReferrer);
rotuerOne.route('/bar', logReferrer);
rotuerTwo.route('/bar', logReferrer);

routerOne.save('/foo');
    // => "http://example.com/"

// Changing the URL to "/bar" causes both routers to dispatch.
routerOne.save('/bar'); 
    // => "http://example.com/foo" (from routerOne)
    // => "http://example.com/"    (from routerTwo)

These two router instances were created at the same time, but their referrers are now out of sync because their routing tables differ, and I don't think this is "correct".

@rgrove
Copy link
Contributor

rgrove commented Sep 17, 2013

It's an interesting conundrum. I can see good reasons for both approaches. Who do we know who works with multiple routers? I don't personally, but I'd love to hear feedback from someone with real world use cases.

@ericf
Copy link
Member

ericf commented Sep 17, 2013

I agree that we should wait for more use cases to serve as data for how to proceed. There are people using multiple routers on the page, and we explicitly support it.

So multiple router people and people who want to ping analytics engines, speak up…

@aronslee
Copy link
Author

Yes Yahoo media sites will have more cases of multiple routers. I.e. movies.yahoo.com

@ericf
Copy link
Member

ericf commented Sep 19, 2013

@aronslee maybe I wasn't clear about my "speak up" comment. What I meant was, based on all the stuff discussed in this thread, we need to know what semantics people are expecting for these complex issues.

@ericf
Copy link
Member

ericf commented Sep 26, 2013

Didn't get any feedback for this for the current sprint so this is not going to land anytime soon and not block #1201.

I think this feature will require a major shift in how Router actually works, more and more it seems we need to separate-out a service which listens for URL changes from the dispatching process.

@ericsoco
Copy link

Reviving this old thread...
Second the request for req.referrer. We need to provide a link to return to the previous page.
Can't speak to the multiple routers perspective, unfortunately.

@ericsoco
Copy link

Bump.

Can't find any way to access the client-side referrer without hacks, but this is critical to implementing back navigation elements. @ericf in the absence of a near-future fix, any other suggestions for accessing the client-side referrer?

@rgrove
Copy link
Contributor

rgrove commented Apr 23, 2014

@ericsoco If all you need to do is navigate to the previous URL in the history stack, window.history.back() will do that.

@ericsoco
Copy link

@rgrove our use case is that we have a set of pages treated as a modal; they each have a close button that should return the user to the previous set of pages. We're actually probably going to take a different route to address this case now, so client referrer is not as critical; however, it would still be a useful feature to surface in the public API.

@rgrove
Copy link
Contributor

rgrove commented Apr 24, 2014

@ericsoco Yep, I agree it would be useful (see my comments above), but the multiple-router issue is a tricky one and would probably require some significant changes, so we should be sure we get it right. In the meantime, there are plenty of ad-hoc workarounds for people who need this functionality, so I don't think there's any rush.

@ericsoco
Copy link

@rgrove works for me, thanks!

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

No branches or pull requests

4 participants