yui / yui3

A library for building richly interactive web applications.
http://yuilibrary.com/
Other
4.12k stars 1.29k forks source link

Router to provide property for document.referrer #1181

Open aronslee opened 11 years ago

aronslee commented 11 years ago

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

ericf commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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

ericf commented 11 years ago

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 commented 11 years ago

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

rgrove commented 11 years ago

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

Why?

ericf commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 10 years ago

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

ericf commented 10 years ago

@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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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

ericsoco commented 10 years ago

@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 commented 10 years ago

@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 commented 10 years ago

@rgrove works for me, thanks!