feat(types): change IData.value type to generic

[NVRC]

No description provided.

[NVRC]

As far as I can tell the commit looks good. Lmk if more testing pipeline stuff needs doing.

$ npm run test && npm run lint && npm run build

> [email protected] test
> jest --config jest.config.js

 PASS  src/utils/getSectionData.test.ts
 PASS  src/components/ListLetterIndex/index.test.tsx
 PASS  src/components/AlphabetList/index.test.tsx

Test Suites: 3 passed, 3 total
Tests:       18 passed, 18 total
Snapshots:   0 total
Time:        3.048s
Ran all test suites.

> [email protected] lint
> eslint


> [email protected] build
> rm -rf dist && tsc

[Kieran-McIntyre]

As far as I can tell the commit looks good. Lmk if you more testing pipeline stuff needs doing.

$ npm run test && npm run lint && npm run build

> [email protected] test
> jest --config jest.config.js

 PASS  src/utils/getSectionData.test.ts
 PASS  src/components/ListLetterIndex/index.test.tsx
 PASS  src/components/AlphabetList/index.test.tsx

Test Suites: 3 passed, 3 total
Tests:       18 passed, 18 total
Snapshots:   0 total
Time:        3.048s
Ran all test suites.

> [email protected] lint
> eslint


> [email protected] build
> rm -rf dist && tsc

Thanks @NVRC! This looks good, will get it merged and will release a new version. The new version will be v2.1.1 👍

[Kieran-McIntyre]

As far as I can tell the commit looks good. Lmk if you more testing pipeline stuff needs doing.

$ npm run test && npm run lint && npm run build

> [email protected] test
> jest --config jest.config.js

 PASS  src/utils/getSectionData.test.ts
 PASS  src/components/ListLetterIndex/index.test.tsx
 PASS  src/components/AlphabetList/index.test.tsx

Test Suites: 3 passed, 3 total
Tests:       18 passed, 18 total
Snapshots:   0 total
Time:        3.048s
Ran all test suites.

> [email protected] lint
> eslint


> [email protected] build
> rm -rf dist && tsc

Thanks @NVRC! This looks good, will get it merged and will release a new version. The new version will be v2.1.1 👍

Sorry, I spoke a little too soon. We're seeing some failing tests, but doesn't look related to your PR. I have a PR to fix this, so will merge my fixes, then you will need to merge master into your branch. Please give me a few minutes to do this.

[NVRC]

@Kieran-McIntyre No rush. I jumped the gun a little with the PR anyways since I didn't add a test case for the complex obj.
I'll do so later tonight if you want to hold off.

[Kieran-McIntyre]

@Kieran-McIntyre No rush. I jumped the gun a little with the PR anyways since I didn't add a test case for the complex obj. I'll do so later tonight if you want to hold off.

@NVRC Okay no worries. I've pushed a new version to master (v3.0.0) if you want to update your PR and add tests etc

[NVRC]

A side effect of this change is that the default sorting behaviour needs some rethinking. With the current assumption that value is a string the library uses it's first character as the index on which to sort.

With another prop we can maintain the existing behaviour and let users define a function to extract the sortable index. This works like a keyExtractor in libraries that consume lists as opposed to maps. I think this is your intention here.

Proposal

... AlphabetListProps ...
  indexExtractor?: (item: IData<Type>) => string;

Default indexExtractor extracts value for backwards compat.

@Kieran-McIntyre Thoughts?

[Kieran-McIntyre]

A side effect of this change is that the default sorting behaviour needs some rethinking. With the current assumption that value is a string the library uses it's first character as the index on which to sort.

With another prop we can maintain the existing behaviour and let users define a function to extract the sortable index. This works like a keyExtractor in libraries that consume lists as opposed to maps. I think this is your intention here.

Proposal

... AlphabetListProps ...
  indexExtractor?: (item: IData<Type>) => string;

Default indexExtractor extracts value for backwards compat.

@Kieran-McIntyre Thoughts?

@NVRC Yes, this looks like a good proposal and would resolve the issue. We will also need to update the README to clarify how this should be used. But happy to go with this 👍