diff --git a/.changeset/slow-eagles-switch.md b/.changeset/slow-eagles-switch.md new file mode 100644 index 00000000..bd147e85 --- /dev/null +++ b/.changeset/slow-eagles-switch.md @@ -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())`. diff --git a/ember-headless-table/package.json b/ember-headless-table/package.json index edcfda83..c7ba1262 100644 --- a/ember-headless-table/package.json +++ b/ember-headless-table/package.json @@ -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", diff --git a/ember-headless-table/src/-private/table.ts b/ember-headless-table/src/-private/table.ts index 86310e1b..2a132fac 100644 --- a/ember-headless-table/src/-private/table.ts +++ b/ember-headless-table/src/-private/table.ts @@ -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'; @@ -52,24 +53,28 @@ export class Table extends Resource> { /** * @private + * + * Unused for now, may be used in the future. + * This data is collected along with the scrollContainerWidth, (which is currently in use) */ @tracked scrollContainerHeight?: number; /** * @private + * + * Used to help determine how much space we can give to columns. + * As we generate widths for columns, the columns' widths must + * add up to about this number. */ @tracked scrollContainerWidth?: number; /** * @private + * + * Lazy way to delay consuming arguments until they are needed. */ @tracked declare args: { named: Signature['Named'] }; - /** - * @private - */ - defaultColumnConfig = DEFAULT_COLUMN_CONFIG; - /** * @private */ @@ -229,10 +234,33 @@ export class Table extends Resource> { 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(this, { ...this.defaultColumnConfig, ...config }); + return new Column(this, { ...DEFAULT_COLUMN_CONFIG, ...config }); }, }); diff --git a/ember-headless-table/types/index.d.ts b/ember-headless-table/types/index.d.ts index 1170faed..437ffd8c 100644 --- a/ember-headless-table/types/index.d.ts +++ b/ember-headless-table/types/index.d.ts @@ -1,3 +1,2 @@ // Installs the types for TS, no-op in JS import 'ember-cached-decorator-polyfill'; - diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d922d751..3d1f24e4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -253,6 +253,7 @@ importers: '@ember/test-waiters': ^3.0.0 '@embroider/addon-dev': ^3.0.0 '@embroider/addon-shim': ^1.0.0 + '@embroider/macros': 1.10.0 '@glimmer/component': ^1.1.2 '@glimmer/tracking': ^1.1.2 '@glint/template': ^0.9.4 @@ -293,6 +294,7 @@ importers: '@babel/runtime': 7.20.1 '@ember/string': 3.0.0 '@embroider/addon-shim': 1.8.3 + '@embroider/macros': 1.10.0 ember-modifier: 3.2.7_@babel+core@7.20.2 ember-resources: 5.6.0_lvuyxhavm3hog3hdunxubajamu ember-tracked-storage-polyfill: 1.0.0 @@ -3160,7 +3162,7 @@ packages: /@types/ember__controller/4.0.3: resolution: {integrity: sha512-7+Lpgcsa/gjGZtUN0JfQoLzUStCT68p8o6t1e4LBXyC+a5DHg2NJYFZ0U3f8F8TgQJcgtFoUAuv8p77XY6FxwQ==} dependencies: - '@types/ember__object': 4.0.5 + '@types/ember__object': 4.0.5_@babel+core@7.20.2 /@types/ember__controller/4.0.3_@babel+core@7.20.2: resolution: {integrity: sha512-7+Lpgcsa/gjGZtUN0JfQoLzUStCT68p8o6t1e4LBXyC+a5DHg2NJYFZ0U3f8F8TgQJcgtFoUAuv8p77XY6FxwQ==} @@ -3339,7 +3341,7 @@ packages: /@types/glob/8.0.0: resolution: {integrity: sha512-l6NQsDDyQUVeoTynNpC9uRvCUint/gSUXQA2euwmTuWGvPY5LSDUu6tkCtJB2SvGQlJQzLaKqcGZP4//7EDveA==} dependencies: - '@types/minimatch': 5.1.2 + '@types/minimatch': 3.0.5 '@types/node': 18.11.9 /@types/htmlbars-inline-precompile/3.0.0: @@ -3378,6 +3380,7 @@ packages: /@types/minimatch/5.1.2: resolution: {integrity: sha512-K0VQKziLUWkVKiRVrx4a40iPaxTUefQmjtkQofBkYRcoaaL/8rhwDWww9qWbrgicNOgnpIsMxyNIUM4+n6dUIA==} + dev: true /@types/minimist/1.2.2: resolution: {integrity: sha512-jhuKLIRrhvCPLqwPcx6INqmKeiA5EWrsCOPhrlFSrbrmU4ZMPjj5Ul/oLCMDO98XRUIwVm78xICz4EPCektzeQ==} @@ -15828,6 +15831,7 @@ packages: '@ember/test-helpers': ^2.6.0 '@ember/test-waiters': ^2.4.5 || ^3.0.0 '@glimmer/component': '*' + '@glimmer/env': ^0.1.7 '@glint/template': '>= 0.8.3' ember-cached-decorator-polyfill: ^1.0.1 ember-source: ^3.28.0 || ^4.0.0 @@ -15866,6 +15870,7 @@ packages: '@ember/test-helpers': ^2.6.0 '@ember/test-waiters': ^2.4.5 || ^3.0.0 '@glimmer/component': '*' + '@glimmer/env': ^0.1.7 '@glint/template': '>= 0.8.3' ember-cached-decorator-polyfill: ^1.0.1 ember-source: ^3.28.0 || ^4.0.0 diff --git a/test-app/tests/unit/table-test.ts b/test-app/tests/unit/table-test.ts index 4ec70a3c..1fa1a4bb 100644 --- a/test-app/tests/unit/table-test.ts +++ b/test-app/tests/unit/table-test.ts @@ -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' + ); + }); });