chore: Move shared table-role and async-store primitives from components - #242
chore: Move shared table-role and async-store primitives from components#242gethinwebster wants to merge 6 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #242 +/- ##
==========================================
- Coverage 99.01% 98.07% -0.94%
==========================================
Files 44 52 +8
Lines 1314 1764 +450
Branches 341 514 +173
==========================================
+ Hits 1301 1730 +429
- Misses 13 33 +20
- Partials 0 1 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…nents@main Pure relocation of src/table/table-role/* and src/area-chart/async-store/* (+ use-previous dep and render-hook test helper), original content and import paths (does not yet satisfy component-toolkit lint by design).
2ce878c to
72a67f9
Compare
There was a problem hiding this comment.
Pull request overview
This PR moves the shared table-role (ARIA role helpers + grid keyboard navigation) and async-store primitives into component-toolkit so multiple table implementations (current Table and upcoming BasicTable/VirtualTable) can reuse them via new internal entrypoints.
Changes:
- Adds the
src/internal/table-rolemodule (helpers, grid navigation processor/provider, and docs) plus unit tests. - Adds the
src/internal/async-storemodule (store + hooks) plus unit tests. - Exposes new package entrypoints
./internal/table-roleand./internal/async-store, and updates unit-test TS libs.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.unit.json | Extends unit-test TS libs to include dom.iterable. |
| package.json | Adds exports entries for ./internal/table-role and ./internal/async-store. |
| src/internal/table-role/utils.ts | Table/grid navigation DOM helpers (cells/rows, focus movement, span handling). |
| src/internal/table-role/table-role-helper.ts | Helpers for applying ARIA/table role props across table structures. |
| src/internal/table-role/interfaces.ts | Shared types for table role + grid navigation props/state. |
| src/internal/table-role/index.ts | Public internal entrypoint re-exporting table-role utilities/provider. |
| src/internal/table-role/grid-navigation.tsx | Grid navigation provider + processor implementing keyboard navigation behavior. |
| src/internal/table-role/grid-navigation.md | Documentation of the grid navigation behavior and patterns. |
| src/internal/table-role/tests/utils.test.ts | Unit tests for navigation utilities (colspan/rowspan + targeting). |
| src/internal/table-role/tests/table-role-helper.test.ts | Unit tests for role prop helpers. |
| src/internal/table-role/tests/stubs.tsx | Test table/controls used by grid navigation tests. |
| src/internal/table-role/tests/grid-navigation.test.tsx | Integration-style tests for keyboard navigation behavior and suppression. |
| src/internal/table-role/tests/grid-navigation-processor.test.tsx | Safety test for processor methods when uninitialized. |
| src/internal/async-store/use-previous.ts | usePrevious hook used by selector logic. |
| src/internal/async-store/index.ts | AsyncStore implementation + useReaction/useSelector hooks. |
| src/internal/async-store/tests/render-hook.tsx | Minimal renderHook test utility for async-store tests. |
| src/internal/async-store/tests/async-store.test.ts | Unit tests validating store subscription + hooks behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/internal/async-store/index.ts:88
usePreviousreturnsundefinedon the first render, so this condition is true on initial mount anduseSelectorreturns a freshly computed selector result instead of the memoized state. If the selector returns non-primitive values, this can cause a reference change without any store change. Check againstundefined(or initializeusePrevioustonull).
if (prevStore !== null && prevStore !== store) {
return selector(store.get());
| // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| import { TableRole } from './interfaces.js'; |
|
|
||
| export type TableRole = 'table' | 'grid' | 'treegrid' | 'grid-default'; |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
src/internal/table-role/grid-navigation.tsx:5
- React hooks are imported from
reactin two separate statements here. Consolidating into a single import keeps the file consistent and avoids duplicate module imports.
import React, { useRef } from 'react';
import { useEffect, useMemo } from 'react';
src/internal/async-store/tests/async-store.test.ts:7
- The test suite name references
AreaChart, but these tests exercise the genericAsyncStoreutilities. Renaming the suite makes failures easier to interpret and avoids stale copy/paste context.
describe('AreaChart AsyncStore', () => {
src/internal/async-store/index.ts:88
usePrevious(store)returnsundefinedon the first render, so theprevStore !== nullguard is always true and this branch will run wheneverprevStoreis stillundefined. If the intent is to synchronously recompute only when the store instance changes, the guard should check forundefined(or a truthy previous value) instead ofnull.
// When store changes we need the state to be updated synchronously to avoid inconsistencies.
const prevStore = usePrevious(store);
if (prevStore !== null && prevStore !== store) {
return selector(store.get());
}
Description
Moves
table-role(grid navigation + ARIA role helpers) andasync-storeout of@cloudscape-design/componentsintocomponent-toolkitso the existing Table and the upcoming BasicTable/VirtualTable can share them.Byte-identical move (no behaviour change); adds two entrypoints:
./internal/table-roleand./internal/async-store.Changes beyond the verbatim move (addressing Copilot review):
table-role/index.ts: re-exportTableRoleas a type-only export (export type { TableRole }) — unambiguous under the ESM build (the value form relied on tsc type-elision).grid-navigation.md: fix a copy/paste typo in thePage Upbullet ("when moving down" → "when moving up"). Pre-existing in componentsmain.Related links, issue #, if available: n/a
How has this been tested?
table-role(grid-navigation,grid-navigation-processor,table-role-helper,utils) andasync-storepass in the toolkit build.@cloudscape-design/componentswas repointed locally to consume these via the new entrypoints; Table suites pass against the shared modules.(Code will be removed from components repo once this package is merged & published, WIP: https://github.com/cloudscape-design/components/pull/4846/changes)
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.