AmpersandJS / ampersand-sync

Provides sync behavior for updating data from ampersand models and collections to the server.
http://ampersandjs.com
MIT License
20 stars 28 forks source link

Set qs.stringify encoding options to create a different query string for array values #80

Closed joseplatas closed 5 years ago

joseplatas commented 5 years ago

If we send a data prop to the fetch call in ampersand-model or ampersand-rest-collection the qs.stringify function encodes the data and updates the options.url string but if we send a data object that has an array prop the encoding will keep the index of the array in the URL string.

But we can overwrite the default behavior of the qs.stringify function by setting options.

var data = {
a:"a",
b:1,
c:["one", "two"]
}

qs.stringify(data);
//outputs => a=a&one=1&status%5B0%5D=one&status%5B1%5D=two

qs.stringify(data, { indices: false });
//outputs => a=a&one=1&status=one&status=two
gdibble commented 5 years ago

@wraithgar Mind taking a look? Thanks old friend :)

wraithgar commented 5 years ago

I haven't been active on this project in years fyi. But this PR was very small and easy to parse and has a test so I feel confident reviewing it.

Other than one minor potential to remove an if statement (which is imo a judgement call, you can ignore my suggestion) it's a +1 from me.

joseplatas commented 5 years ago

@dhritzkiv @wraithgar Out of curiosity, who is the person in charge to merge this PR and update the ampersand-model and ampersand-rest-collection?

dhritzkiv commented 5 years ago

@joseplatas It will be me; I will merge this PR and update the other repos and packages.

I'll do this sometime this weekend

joseplatas commented 5 years ago

@dhritzkiv Thank you! 👍

dhritzkiv commented 5 years ago

Published on npm as 5.1.0

dhritzkiv commented 5 years ago

@joseplatas It looks like ampersand-model and ampersand-rest-collection should target ampersand-sync@5.1.0 as per the ^5.0.0 semver target. You can either re-install those two packages, or add ampersand-sync as a explicit dependency to your project.

joseplatas commented 5 years ago

@dhritzkiv Thank you!!