Skip to content

Conversation

@jordansoltman
Copy link

I realize this pull request is attempting a huge amount of changes in one go, but I'll submit the request for your consideration regardless if you choose to merge.

This restructures the library, and adds some major functionality while maintaining backwards compatibility.

Additions / Bug Fixes

  • TypeScript
    The project has been converted to TypeScript to simplify development, and as a side-effect produce a TypeScript definition file. npm run build builds the source to lib/. eslint is abandoned for tslint for the source files, still run with npm run lint. eslint is however still used for the spec files until (if) they ever get converted to TypeScript, now linted with npm run lint-specs.

  • Composite Keys
    My main purpose for forking was to add support for composite keys. For example, below both a and b together form the primary key of my data. There was no way to get a result shown below:

var table = [
	{ a: '1', b: '1' },
	{ a: '1', b: '2' },
	{ a: '2', b: '1' },
	{ a: '2', b: '2'}
];
var definition = [{
	a: { column: 'a', id: true },
	b: { column: 'b', id: true }
}];
// RESULT
[ { a: '1', b: '1' },
  { a: '1', b: '2' },
  { a: '2', b: '1' },
  { a: '2', b: '2' } ]

This also happens to fix a bug that exists in the current version. If the key for the root isn't the first column, it fails:

var definition = {
    a: 'a',
    b: { column: 'b', id: true } // Creates error
}
  • Arrays

I also wanted to tackle the array issue #13 so I added support for an array flag that can be set to true. This will then cause the values for that column to aggregate in an array.

var table = [
	{ id: 1, foo: 'foo' },
	{ id: 1, foo: 'bar' },
	{ id: 1, foo: 'baz' },
	{ id: 2, foo: 'foo' }
];
var definition = [{
	id: 'id',
	title: {column: 'foo', array: true},
}];
// RESULT
[
	{
        id: 1,
        title: ['foo', 'bar', 'baz']
    },
    {
        id: 2,
        title: ['foo']
    }
]

Testing

I added more tests, and all existing tests pass.

@jordansoltman
Copy link
Author

I removed Node 0.X from travis as jasmine doesn't support running tests on it anymore: https://github.com/jasmine/jasmine/blob/master/release_notes/3.0.md

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.

1 participant