Skip to content

Commit

Permalink
Throw an error when duplicate keys are found.
Browse files Browse the repository at this point in the history
+ Add test
+ Changeset
+ Refresh lockfile from main
+ Switch to esm-env instead of @glimmer/env
+ Instead of introducing a new dependency to the ecosystem, use @embroider/macros
  • Loading branch information
NullVoxPopuli committed Dec 18, 2022
1 parent 325e6ff commit 57c22d4
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 6 deletions.
11 changes: 11 additions & 0 deletions .changeset/slow-eagles-switch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
'ember-headless-table': patch
---

Prevent hard-to-debug issues that occur with incorrect column configs.
One such way problems can occur is when the `key` property is duplicated
for multiple column configs.

This is now eagerly prevented via dev-time Error.
All the column config validity checking code is removed in production builds
via `@embroider/macros` `macroCondition(isDevelopingApp())`.
1 change: 1 addition & 0 deletions ember-headless-table/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
"@babel/runtime": "^7.17.8",
"@ember/string": "^3.0.0",
"@embroider/addon-shim": "^1.0.0",
"@embroider/macros": "1.10.0",
"ember-modifier": "^3.2.7",
"ember-resources": "^5.4.0",
"ember-tracked-storage-polyfill": "^1.0.0",
Expand Down
26 changes: 25 additions & 1 deletion ember-headless-table/src/-private/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { assert } from '@ember/debug';
import { action } from '@ember/object';
import { guidFor } from '@ember/object/internals';

import { isDevelopingApp, macroCondition } from '@embroider/macros';
import { modifier } from 'ember-modifier';
import { Resource } from 'ember-resources/core';
import { map } from 'ember-resources/util/map';
Expand Down Expand Up @@ -234,7 +235,30 @@ export class Table<DataType = unknown> extends Resource<Signature<DataType>> {

if (!configFn) return [];

return configFn() ?? [];
let result = configFn() ?? [];

if (macroCondition(isDevelopingApp())) {
/**
* Assertions for a column config to be valid:
* - every key must be unique
*/
let keys = new Set();
let allKeys = result.map((columnConfig) => columnConfig.key);

result.forEach((columnConfig) => {
if (keys.has(columnConfig.key)) {
throw new Error(
`Every column key in the table's column config must be unique. ` +
`Found duplicate entry: ${columnConfig.key}. ` +
`All keys used: ${allKeys}`
);
}

keys.add(columnConfig.key);
});
}

return result;
},
map: (config) => {
return new Column<DataType>(this, { ...DEFAULT_COLUMN_CONFIG, ...config });
Expand Down
1 change: 0 additions & 1 deletion ember-headless-table/types/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
// Installs the types for TS, no-op in JS
import 'ember-cached-decorator-polyfill';

13 changes: 9 additions & 4 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 21 additions & 0 deletions test-app/tests/unit/table-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,25 @@ module('Unit | -private | table', function (hooks) {
assert.strictEqual(table.columns[position]?.key, key);
});
});

test('columns: each key must be unique', async function (assert) {
const table = headlessTable(this, {
columns: () =>
[
{ key: 'firstName', name: 'First name' },
{ key: 'role', name: 'Role' },
{ key: 'favouritePet', name: 'Favourite Pet' },
{ key: 'firstName', name: 'Last name (typo)' },
] as ColumnConfig[],
data: () => [],
});

assert.throws(
() => {
table.columns.values();
},
/Every column key in the table's column config must be unique. Found duplicate entry: firstName/,
'expected error received'
);
});
});

0 comments on commit 57c22d4

Please sign in to comment.