Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add comma format to the arrayFormat option #167

Merged
merged 6 commits into from Mar 14, 2019

Conversation

zepod
Copy link
Contributor

@zepod zepod commented Mar 1, 2019

This pull requests adds fourth arrayFormat option, named comma. We needed this for our project, as we needed to support arrays in query in below shape...

foo=1,2,3,4,5,6,7,8

This format is mainly used to shorten URLs which include things as identifiers, where there is expected to be a lot of them.

On the way to add this feature I had to slightly reimplement encoderForArrayFormat to use Array.prototype.reduce instead of previous for .. in

@tomaass
Copy link

tomaass commented Mar 4, 2019

Great work @zepod !
Fix tests please I really want to use it on my project 🥇

@sindresorhus
Copy link
Owner

Why did you have to switch to Array#reduce?

@sindresorhus
Copy link
Owner

You have to include TypeScript types for the changes too, since a TypeScript declaration file was recently added to master.

@zepod
Copy link
Contributor Author

zepod commented Mar 4, 2019

@sindresorhus
the way it was implemented with for encoderForArrayFormat was a unary function mapping single element of provided array, to single key in the query sequence.

Using comma format, we wanted to map the multiple elements of array to a single key in the query.

.. Typescript types I will deliver

@zepod
Copy link
Contributor Author

zepod commented Mar 8, 2019

Typescript types added

@sindresorhus sindresorhus changed the title Add comma arrayFormat Add comma format to the arrayFormat option Mar 14, 2019
@sindresorhus sindresorhus merged commit b22c2af into sindresorhus:master Mar 14, 2019
@sindresorhus
Copy link
Owner

Looks good. Thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants