Skip to content

fix: convert notification store to thunks - #633

Open
KhushalSainS wants to merge 1 commit into
WordPress:developfrom
KhushalSainS:fix/client-store-bugs-and-thunks
Open

KhushalSainS wants to merge 1 commit into
WordPress:developfrom
KhushalSainS:fix/client-store-bugs-and-thunks

Conversation

@KhushalSainS

Copy link
Copy Markdown

What?

Fix the core/wp-notifications data store and replace the broken
controls/resolvers with thunks. Add a typed REST→client normalisation layer and make
the reducer safe for unregistered contexts and unknown ids.

Why?

The store never received fetched notifications and would crash on common operations
once the REST endpoint starts returning data. The bugs were invisible only because
the endpoint currently returns []:

  • resolvers.fetchUpdates returned hydrate() instead of dispatching it, so a
    resolver's return value was discarded and nothing entered the store.
  • registerContext was a selector that mutated state in place (called via select),
    so subscribers were never notified.
  • HYDRATE/ADD spread a possibly-missing state[context], throwing a TypeError
    for any unregistered context.
  • findContext returned undefined on no match, so DELETE/UPDATE indexed
    state[undefined] (a double dismissal triggered it).
  • CLEAR mutated the previous state before spreading.
  • controls.FETCH read notice.date, but the REST schema emits created_at.
  • selectors.fetchUpdates returned the whole state just to trigger the resolver.

Fixes #619.

How?

  • Delete src/store/controls.ts and src/store/resolvers.ts and replace them with a
    fetchUpdates thunk (thunks are supported by default in @wordpress/data 10.x).
    Concurrent dispatches are de-duplicated so overlapping resolver calls issue one request.
  • Add a typed src/store/normalize.ts mapping the REST payload to the client Notice:
    created_at → date, is_dismissible → dismissible, channel_title → source,
    icon → { dashicons }, and the accept_link/accept_label/dismiss_label action fields.
  • Move registerContext to an action plus a REGISTER_CONTEXT reducer case, and dispatch
    it from wp-notifications.ts.
  • Make the reducer resilient: handle unregistered contexts, ignore unknown ids, stop
    CLEAR mutating, and upsert by id using an index Map (linear merge).
  • Add one real resolver on getNotices( context ) so the first read of a context fetches.
  • Guard findContext/findNotice/getNotices against missing data.
  • Add unit tests reproducing each bug, and allow the @jest-environment JSDoc tag in the
    tests/** ESLint override.

npm run test:js is intentionally left as-is (still stubbed) in this repo, so the new tests
run manually rather than in CI.

Testing Instructions

  1. Install dependencies with npm ci.
  2. Run the new unit tests with npx jest --selectProjects unit.
  3. Confirm all 21 tests pass, covering: HYDRATE into an unregistered context,
    DELETE of an unknown id, CLEAR not mutating, created_at → date, and the
    fetchUpdates thunk hydrating the store (with concurrent-fetch de-duplication).
  4. Run npm run lint:js and confirm no errors.

Use of AI Tools

AI assistance: Yes
Tool: OpenAI Codex

@jonathanbossenger jonathanbossenger mentioned this pull request Oct 16, 2026
2 of 22 tasks

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix client store bugs and convert to thunks

1 participant