Skip to content

Avoid sparse arrays and serialize as hash unless we have sequential arrays #33

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.

Sign up for GitHub

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

Open
Seldaek wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Avoid sparse arrays and serialize as hash unless we have sequential arrays #33

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f03bf1
Avoid sparse arrays and serialize as hash unless we have sequential a…
Seldaek Apr 10, 2016
743205d
Fix range operator for consistency
Seldaek Apr 10, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,10 @@ function hash_assign(result, keys, value) {

// If the characters between the brackets is not a number it is an
// attribute name and can be assigned directly.
if (isNaN(index)) {
//
// If we process the first record and it is non-zero we assume it's a sparse array
// and use a hash instead of an array to avoid very large sparse arrays
if (isNaN(index) || (result === undefined && index !== 0)) {
result = result || {};
result[string] = hash_assign(result[string], keys, value);
}
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"test": "test"
},
"devDependencies": {
"core-assert": "^0.1.3",
Copy link
Owner

@defunctzombie defunctzombie Apr 10, 2016

Choose a reason for hiding this comment

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

incorrect dependency specification

Copy link
Author

@Seldaek Seldaek Apr 10, 2016

Choose a reason for hiding this comment

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

Is it? Without that deepStrictEqual is not defined because zuul depends on the assert package which is not up to date with node's assert (https://nodejs.org/api/assert.html).

`-- [email protected]
  `-- [email protected]
    `-- [email protected]

It seems like the assert package is yours as well though so maybe you can tell me more about why it's missing deepStrictEqual :)

And BTW in case it's not clear, I need deepStrictEqual because otherwise an object with the same shape as an array is considered equal to the array, which is not quite valid.

Copy link
Owner

@defunctzombie defunctzombie Apr 10, 2016

Choose a reason for hiding this comment

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

Incorrect in the sense that you did not use ~ like the other dependencies do. When in doubt look at adjacent code/style and be consistent with it leaving whatever your other projects or preferences may be out.

Copy link
Author

@Seldaek Seldaek Apr 10, 2016

Choose a reason for hiding this comment

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

Oh that, sorry yeah I just did it from CLI with npm install --save-dev and let it pick the dependency. I'll update although in this case ~ or ^ will behave strictly equally AFAIK.

"domify": "~1.4.0",
"zuul": "~3.10.1"
},
Expand All @@ -27,4 +28,4 @@
"bugs": {
"url": "https://github.com/shtylman/form-serialize/issues"
}
}
}
62 changes: 53 additions & 9 deletions test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,26 @@
var assert = require('assert');
var assert = require('core-assert');
var domify = require('domify');

var serialize = require('../');

var hash_check = function(form, exp) {
assert.deepEqual(serialize(form, { hash: true }), exp);
assert.deepStrictEqual(serialize(form, { hash: true }), exp);
};

var str_check = function(form, exp) {
assert.equal(serialize(form), exp);
};

var disabled_check = function(form, exp) {
assert.deepEqual(serialize(form, { hash : false, disabled: true }), exp);
assert.deepStrictEqual(serialize(form, { hash : false, disabled: true }), exp);
};

var empty_check = function(form, exp) {
assert.deepEqual(serialize(form, { hash : false, disabled: true, empty: true }), exp);
assert.deepStrictEqual(serialize(form, { hash : false, disabled: true, empty: true }), exp);
};

var empty_check_hash = function(form, exp) {
assert.deepEqual(serialize(form, { hash : true, disabled: true, empty: true }), exp);
assert.deepStrictEqual(serialize(form, { hash : true, disabled: true, empty: true }), exp);
};

test('null form', function() {
Expand Down Expand Up @@ -426,6 +426,20 @@ test('bracket notation - non-indexed arrays', function() {
});
});

test('bracket notation - sparse array', function() {
var form = domify('<form>' +
'<input name="user[1234]" value="cow" />' +
'<input name="user[4567]" value="milk" />' +
'</form>');

hash_check(form, {
user: {
1234: "cow",
4567: "milk",
}
});
});

test('bracket notation - nested, non-indexed arrays', function() {
var form = domify('<form>' +
'<input name="user[tags][]" value="cow" />' +
Expand All @@ -452,19 +466,49 @@ test('bracket notation - indexed arrays', function() {
'</form>');

hash_check(form, {
people: [
{
people: {
0: {
name: "fred",
age: "12"
},
{
1: {
name: "bob",
age: "14"
},
{
2: {
name: "bubba",
age: "15"
},
3: {
age: "2"
},
_values: [
{name: "frank"}
]
}
});
});

test('bracket notation - ordered indexed arrays', function() {
var form = domify('<form>' +
'<input name="people[0][name]" value="fred" />' +
'<input name="people[0][age]" value="12" />' +
'<input name="people[1][name]" value="bob" />' +
'<input name="people[1][age]" value="14" />' +
'<input name="people[][name]" value="frank">' +
'<input name="people[2][age]" value="2">' +
'</form>');

hash_check(form, {
people: [
{
name: "fred",
age: "12"
},
{
name: "bob",
age: "14"
},
{
name: "frank",
age: "2"
Expand Down