Skip to content

RAW Body & HTML 5 Files API#2120

Closed
ahmadnassri wants to merge 3 commits intomootools:masterfrom
codeinchaos:master
Closed

RAW Body & HTML 5 Files API#2120
ahmadnassri wants to merge 3 commits intomootools:masterfrom
codeinchaos:master

Conversation

@ahmadnassri
Copy link

this allows to send a RAW request body and adds the ability to upload files (HTML5)

you probably might choose to separate into separate classes (in More perhaps?) I just thought this is a much needed basic functionality that should be part of Core (or More) MooTools, not as a plugin ...

can send RAW request body
HTML5 Files upload
added "application/json" to list of Accept header
can send RAW request body
HTML5 Files upload
@Inviz
Copy link

Inviz commented Nov 6, 2011

I like the stuff, but keep in mind, it's unsafe to change "Accept" headers, as there are plenty of web resources (e.g. everything built on rails) that use content type negotiation and the change would make it recieve a json response to plain request by default.

Personally, i prefer the same stuff, json over html. But the application has to be tuned to do it. Putting this kind of incompatability in defaults is dangerous and will create problems

@Inviz
Copy link

Inviz commented Nov 6, 2011

And yeah, those code comments were better on github, not in source code. but that's just my opinion.

@ahmadnassri
Copy link
Author

I hear your point about the Accept header, though my core belief is to stick to the standard, and since there is none for default Accept header, its really up to the implementer to decide, in this case, whoever is writing the javascript functions should overwrite to match his use-case...

that said, I realize that there are many "novice" devs out there who don't even understand any of this and just assume it will work ... I have a chrome extension project REST Console built with MooTools and people keep complaining that POST requests don't work, after much investigation turns out they were not setting the Content-Type header to application/x-www-form-urlencoded ... nobody even knew what it is!

so yeah, you probably want to change those headers back ...

Copy link
Author

Choose a reason for hiding this comment

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

there should be a better way to distinguish between POST data and Query String parameters, its really inefficient they way it is now, as you should be able to send both Query Params and POST data at the same time ...

I know this is doable by modifying the URL field, but it would be nice if it can be set as an object.

perhaps that would be in my next pull request .. if you guys approve?

Copy link

Choose a reason for hiding this comment

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

Perhaps that only matters in environments that make difference between GET & POST params (e.g. rails does not care, as it puts everything into params magic hash). Working with rails, i find it convenient to be able to provide all kind of params in a single hash (e.g. to send additional authenticity_token param easily).

So if one wants his parameter sent in query string, he should use a custom subclass of Request that would accept an array of param names to put them into query string, instead of post body.

I see no benefit in dealing with that stuff manually, especially per request.

Copy link
Author

@ahmadnassri ahmadnassri Nov 10, 2011

Choose a reason for hiding this comment

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

I disagree, as a library, MooTools should be platform agnostic and follow the HTTP Spec RFC 2616 that said, I understand that 99.9% of developers don't even understand how HTTP works and just rely on their environment's way of doing things (Rails, PHP, etc ..) so perhaps the answer is to create a Request.REST Class that is the parent class for all Reqest.* Classes and the current Request Class simply makes things simpler for people using these environments (that clearly do not follow the HTTP Spec) by allowing such behaviors as currently implemented.

Copy link

Choose a reason for hiding this comment

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

Well, it doesn't mean it has to be able to do everything either. Inheritance is a great way to customize the classes you want. Get a good javascript packaging system, and deployment of customized mootools builds will be as easy, as if it was merged together.

Copy link
Author

Choose a reason for hiding this comment

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

I guess you missed my point about the Library not doing specialized work ...

I already have my inheritance and overwrites for my own projects going, I was suggesting that the core library should stick to standards and in this case provide a real HTTP Request class ...

you said it yourself: some environments don't make difference between GET & POST params, and that would be a perfect example where one would create inherited classes and get a packaging system going ... which is why the core library should always adhere to standards ... in this case, query string and post params are completely different elements of a request object and should be treated as such ...

@ahmadnassri ahmadnassri closed this Sep 2, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants