google / google/site-kit-wp

Type `createTestRegistry` with the shared `Registry` type

Open
#13,403 0 comments 0 reactions 1 assignee Claimed by @tofumatt View on GitHub
P2 Team S Type: Infrastructure
Dominant language
JavaScript
Stars
1.4k
Forks
383
Avg merge
4d 14h
Merged PRs (30d)
77

Description

## Feature Description

`createTestRegistry()` in `tests/js/utils.ts` returns `createRegistry()` from `@wordpress/data`, which is typed as `WPDataRegistry`. That type has no `resolveSelect`, even though the registry it creates does have it at runtime. Any test that needs `resolveSelect`, for example one passing a registry into a notification's `checkRequirements`, has to widen the type itself before the code will compile.

We already have a type for this. `Registry` in `assets/js/googlesitekit/data/types.ts` is `WPDataRegistry` plus `resolveSelect`, and it is exported from `googlesitekit-data`. `createTestRegistry` should return it, so that individual test files stop redefining it.

This is its own ticket rather than a one-line change because `render` and `renderHook` in `tests/js/test-utils.tsx` take their `registry` types from `ReturnType< typeof createTestRegistry >`. Correcting the return type therefore stops every test that declares its own registry as `WPDataRegistry` from compiling, currently around 250 typecheck errors across 30 test files. The work is mechanical and carries no runtime change, but it is too wide to land inside a feature pull request.

This is a follow-up to [a review comment on #13368](https://github.com/google/site-kit-wp/pull/13368#discussion_r3814527559), which asked for this ticket.

---------------

_Do not alter or remove anything below. The following sections will be managed by moderators only._

## Acceptance criteria

* `createTestRegistry()` returns the shared `Registry` type, which includes `resolveSelect`.
* Test files that declare the type of their registry use the shared `Registry` type rather than WordPress's `WPDataRegistry`.
* Test and Storybook files that declare their own local copy of the registry type use the shared type instead.
* Casts that exist only to add `resolveSelect` to the result of `createTestRegistry()` are removed.
* The project typecheck passes with no registry type errors remaining.
* The existing JavaScript test suite passes unchanged.

## Implementation Brief

* [ ] In `tests/js/utils.ts`:
* Declare the return type of `createTestRegistry` as `Registry`, imported from `googlesitekit-data`, and cast the `createRegistry()` result to it.

* [ ] Across the test files that stop compiling as a result, currently around 250 errors in 30 files, all reported by the project typecheck:
* Replace each `registry: WPDataRegistry` annotation with `Registry`, and replace the `@wordpress/data/build-types/registry` import with `Registry` from `googlesitekit-data`. Each file has exactly one import and one annotation to change. They sit under `assets/js/components/`, `assets/js/feature-tours/`, `assets/js/hooks/`, and the `adsense`, `analytics-4`, `reader-revenue-manager` and `search-console` module directories.

* [ ] Across the files that already cast, 19 occurrences of `createTestRegistry() as Registry` in 15 files:
* Remove the `as Registry` cast and keep the `Registry` annotation on the variable.

* [ ] In the five files declaring `type Registry = ReturnType< typeof createTestRegistry >;`, namely `assets/js/components/pdf-export/test-utils.ts`, `assets/js/modules/reader-revenue-manager/components/dashboard/ExpressSetupResumeNewsletterNotification/index.stories.tsx`, `assets/js/modules/reader-revenue-manager/components/dashboard/ExpressSetupResumeNotice/index.stories.tsx`, `assets/js/modules/reader-revenue-manager/components/dashboard/PolicyViolationNotification/index.stories.tsx` and `assets/js/modules/reader-revenue-manager/components/common/PolicyViolationSettingsNotice.stories.tsx`:
* Delete the local alias and import `Registry` from `googlesitekit-data`.

* [ ] Settle on `googlesitekit-data` as the single import path for `Registry`, and update the files that currently reach for `@/js/googlesitekit-data` or `@/js/googlesitekit/data/types` instead.

The `getPDFData` test files declare their own `Registry` that intersects `WPDataRegistry` with the PDF registry shape. That is a different type and stays as it is.

### Test Coverage

* No new test cases. The change is confined to types and does not alter runtime behaviour, so the existing JavaScript suite is the check: it must pass unchanged, and the project typecheck must report no remaining registry type errors.

## QA Brief

*

## Changelog entry

*

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.