Skip to content
This repository has been archived by the owner on Jul 30, 2018. It is now read-only.

Adding a flag to disable automatic inclusion of x-requested-with #350

Merged
merged 3 commits into from
Aug 1, 2017

Conversation

rorticus
Copy link
Contributor

Type: feature

The following has been addressed in the PR:

  • There is a related issue
  • All code matches the style guide
  • Unit or Functional tests are included in the PR

Description:

Adding an XHR request option, includeRequestedWith that defaults to true, that can be used to disable the automatic inclusion of the X-Requested-With.

Example,

xhr('some-url', { includeRequestedWith: false }).then(...)

Resolves #328

@rorticus rorticus requested a review from kitsonk July 31, 2017 17:59
@@ -18,6 +18,7 @@ import SubscriptionPool from '../SubscriptionPool';
*/
export interface XhrRequestOptions extends RequestOptions {
blockMainThread?: boolean;
includeRequestedWithHeader?: boolean;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only concern with this is that in general we don't have good documentation for any of this, and now we are adding to it. Without any JSDoc, the API docs won't contain anything as well. At the very least, we should add JSDoc for options we add, we already have a cleanup task to go back over and review stuff, so lets just not add to the pile of debt.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point Kit, we should definitely be looking to at least have JSDoc (and even better README doc too)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was going to say README, but realise for core that is a ginormous task and actually for lots of it, just sane API docs is likely to be better, maybe... Adding this particular flag to the README would be a bit crazy as it stands at the moment, since this is our request documentation at the moment:

HTTP requests

The @dojo/core/request module contains methods to simplify making http requests. It can handle making requests in both node and the browser through the same methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some jsdoc for request options 😄

@rorticus rorticus merged commit 5448b09 into dojo:master Aug 1, 2017
@rorticus rorticus deleted the issue-328 branch August 1, 2017 17:29
@dylans dylans added this to the 2017.08 milestone Aug 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants