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

Introduces new `mapKeys` option #293

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@aaronsky
Copy link

aaronsky commented Jan 2, 2019

Fixes #291

This patch introduces a new option for the stringify and parse
functions to apply a transformation per key before encoding/after
decoding.

This is still a WIP as I can't seem to get the test suite
passing locally. Any and all feedback would be appreciated!

aaronsky added some commits Jan 2, 2019

Introduces new `mapKeys` option
Fixes #291

This patch introduces a new option for the `stringify` and `parse`
functions to apply a transformation per key before encoding/after
decoding.
"multiline-comment-style": 0,
"no-continue": 1,
"no-magic-numbers": 0,
"no-restricted-syntax": [2, "BreakStatement", "DebuggerStatement", "ForInStatement", "LabeledStatement", "WithStatement"],
"operator-linebreak": [2, "before"],
"operator-linebreak": [2, "before"]

This comment has been minimized.

@ljharb

ljharb Jan 3, 2019

Owner

trailing commas are fine in eslintrc files, fwiw

Suggested change Beta
"operator-linebreak": [2, "before"]
"operator-linebreak": [2, "before"],
@@ -409,6 +418,15 @@ assert.equal(
);
```

In addition, you can transform keys entirely without writing a custom encoder using `mapKeys`:

This comment has been minimized.

@ljharb

ljharb Jan 3, 2019

Owner

how does this interact with a custom encoder? are the options mutually exclusive, or is there a specific order they run in? (same question re a custom decoder in parse)

@@ -578,7 +578,7 @@ var merge = function merge(target, source, options) {
if (Array.isArray(target) && Array.isArray(source)) {
source.forEach(function (item, i) {
if (has.call(target, i)) {
if (target[i] && typeof target[i] === 'object') {
if (target[i] && typeof target[i] === 'object' && item && typeof item === 'object') {

This comment has been minimized.

@ljharb

ljharb Jan 3, 2019

Owner

please revert this; the dist file should only be updated at release time.

Suggested change Beta
if (target[i] && typeof target[i] === 'object' && item && typeof item === 'object') {
if (target[i] && typeof target[i] === 'object') {
@@ -78,6 +81,8 @@ var parseValues = function parseQueryStringValues(str, options) {
val = options.decoder(part.slice(pos + 1), defaults.decoder, charset);
}

key = options.mapKeys(key);

This comment has been minimized.

@ljharb

ljharb Jan 3, 2019

Owner

this should be extracted out so that the mapKeys function does not get options as its receiver (this value)

@@ -87,9 +92,9 @@ var stringify = function stringify( // eslint-disable-line func-name-matching

var objKeys;
if (Array.isArray(filter)) {
objKeys = filter;
objKeys = filter.map(mapKeys);

This comment has been minimized.

@ljharb

ljharb Jan 3, 2019

Owner

this will cause three arguments to be passed to mapKeys, perhaps we want function (k) { return mapKeys(k); } to limit the observable API?

} else {
var keys = Object.keys(obj);
var keys = Object.keys(obj).map(mapKeys);

This comment has been minimized.

@ljharb

ljharb Jan 3, 2019

Owner

same here

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