Skip to content
This repository was archived by the owner on Mar 13, 2018. It is now read-only.

Conversation

@benzen
Copy link

@benzen benzen commented Sep 12, 2014

convert each value candidate to string params into a json string before
go into encoreURIComponent

Used JSON.stringify, which according to http://caniuse.com/#search=JSON
is available in every browser without restriction since IE9

Added test for that, but can't run them since there is missing parts for
running them from the sources

convert each value candidate to string params into a json string before
go into encoreURIComponent

Used JSON.stringify, which according to http://caniuse.com/#search=JSON
is available in every browser without restriction since IE9

Added test for that, but can't run them since there is missing parts for
running them from the sources
@benzen
Copy link
Author

benzen commented Sep 12, 2014

This PR seems to treat the same aspect as #21.
But #21 take into account only arrays.

In my opinion, this one is more simple and straight forward.

Also i've created an issue for this last week, #24.

@benzen
Copy link
Author

benzen commented Sep 12, 2014

My bad, i didn't see the bower.json.
I'll do my homework

Separate tests for neasted object and arrays into distinct files
correct xhr parm encoding
@benzen
Copy link
Author

benzen commented Sep 12, 2014

this time the test have been runned and it works

@benzen
Copy link
Author

benzen commented Sep 12, 2014

I'v signed the cla also

@urandom
Copy link

urandom commented Sep 23, 2014

@benzen
PR #21 (I'm the author of that one), deals with a completely different problem. It adds support for creating same-named parameters, which is a very mature standard. Using array properties of the params object is just for convenience, since core-ajax uses a an object for those. And there is currently no other way to create these parameters.

This PR is in fact conflicting, since it removes that capability. It is also very specific, since no one has standardized that the parameter values can be json, and server-side code needs to add code to decode the parameter values it expects to be in json. Furthermore, the parameter values can be encoded before the params object is passed to core-ajax, thus while support for encoding complex objects into json strings is quite convenient in core-ajax, it might not be the best place for it (or at least shouldn't happen automatically).

@benzen
Copy link
Author

benzen commented Sep 23, 2014

Sorry i didnt give enough Time to reading your PR.
The problem with The current implementation is that core Ajax expect object and xhr expect string.
So the paramètre you give will change under your feet.

I think we should give it more Time in order to merge ouR PR.

Envoyé de mon iPhone

Le 2014-09-23 à 03:22, Viktor Kojouharov notifications@github.com a écrit :

@benzen
PR #21 (I'm the author of that one), deals with a completely different problem. It adds support for creating same-named parameters, which is a very mature standard. Using array properties of the params object is just for convenience, since core-ajax uses a an object for those. And there is currently no other way to create these parameters.

This PR is in fact conflicting, since it removes that capability. It is also very specific, since no one has standardized that the parameter values can be json, and server-side code needs to add code to decode the parameter values it expects to be in json. Furthermore, the parameter values can be encoded before the params object is passed to core-ajax, thus while support for encoding complex objects into json strings is quite convenient in core-ajax, it might not be the best place for it (or at least shouldn't happen automatically).


Reply to this email directly or view it on GitHub.

@garlicnation
Copy link
Contributor

Might it be more appropriate to send this JSON in the body of a POST request? Alternatively, you can stringify your json before setting the value of the parameter. The decision on how to handle object-valued parameters seems like something that should be handled on a per-application basis, instead of being baked into core-ajax.

@benzen
Copy link
Author

benzen commented Oct 31, 2014

I do agree with this proposition.
But core-ajax expect an object and core xhr expect a string So i should lpose the benefits of core-ajax just for this feature
Seams weird to me

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants