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

fix issue 210: custom values formatting schema #249

Closed
wants to merge 8 commits into from

Conversation

goodwin64
Copy link

Resolves #210

@@ -250,7 +279,9 @@ function parse(input, options) {
return ret;
}

return (options.sort === true ? Object.keys(ret).sort() : Object.keys(ret).sort(options.sort)).reduce((result, key) => {
Copy link
Author

Choose a reason for hiding this comment

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

Too much logic in the return statement makes it hard to understand what's actually returned from the function.

index.d.ts Outdated Show resolved Hide resolved
if (typeof typedSchemaFormatter === 'function') {
return typedSchemaFormatter;
}

Copy link
Owner

Choose a reason for hiding this comment

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

It should throw if the typedSchemaFormatter type is not a supported type.

baz: 3
};
t.deepEqual(actual, expected);
});
Copy link
Owner

Choose a reason for hiding this comment

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

This feature will need a lot more tests.

test/parse.js Outdated Show resolved Hide resolved
```
*/
readonly types?: {
[key: string]: 'string' | 'number' | CustomValueParser;
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
[key: string]: 'string' | 'number' | CustomValueParser;
[type: string]: 'string' | 'number' | CustomValueParser;

queryString.parse('foo=1&bar=1&baz=1.5', {
types: {
foo: 'string',
bar: 'number',
Copy link
Owner

Choose a reason for hiding this comment

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

What about an array of string or array of numbers? We should probably use the notation string[] and number[] for those.

Copy link
Author

Choose a reason for hiding this comment

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

Agree

goodwin64 and others added 2 commits March 2, 2020 19:17
Co-Authored-By: Sindre Sorhus <sindresorhus@gmail.com>
Co-Authored-By: Sindre Sorhus <sindresorhus@gmail.com>
@goodwin64
Copy link
Author

Sorry, I've just realized unfortunately I have no spare time to finish the PR. If someone is willing to finish - it'd cool to finally deliver this functionality.

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.

Support predefining types
2 participants