search

youtube / spfjs

A lightweight JS framework for fast navigation and page updates from YouTube
https://youtube.github.io/spfjs/
MIT License
2.23k stars 147 forks source link

Allow setting withCredentials in spf xhr options #417

Closed zhaoz closed 8 years ago

zhaoz commented 8 years ago

This allows SPF to be used with CORS + Cookies.

nicksay commented 8 years ago

You're going to have to thread this to spf.nav.request.send as well. To make this available to clients, we could also thread this one step further to spf.nav.load. (Since withCredentials has no-effect on same-domain requests and we currently force reloads on cross-origin navigation, I don't think we need to make this available to spf.nav.navigate.)

zhaoz commented 8 years ago

Added commits to forward withCredentials to spf.nav.request.send and spf.nav.load, also added to the markdown file.

Let me know if you prefer to have this squashed into one commit.

nicksay commented 8 years ago

Please do squash into a single commit. You either do it beforehand in your fork or "squash and merge" using the GitHub UI when merging the PR.

zhaoz commented 8 years ago

Squashed my commits and addressed comments. Not sure how to mark comments as addressed though.

nicksay commented 8 years ago

Ah, sorry I forgot this before -- can you add the withCredentials property to the typedef in base.js too? https://github.com/youtube/spfjs/blob/master/src/client/base.js#L301

zhaoz commented 8 years ago

Updated

rviscomi commented 8 years ago

LGTM after nits.

zhaoz commented 8 years ago

Addressed comment issue. And updated.