chore: bump @ably/ui from 17.14.0 to 18.3.1#3418
Conversation
Catches docs up to the @ably/ui line shipped on the main marketing site and Voltaire. The headline reason for moving now is the Mixpanel EU-ingestion default that landed in 18.3.1: Mixpanel sunsets its US→EU forwarding bridge on 2026-07-01, and 18.3.1 defaults `mixpanel.init` to `api_host: https://api-eu.mixpanel.com` so we route directly to the EU project instead of relying on the soon-to-be-removed forwarder. This crosses a major boundary (17 → 18), so the design system surface this app consumes (typography utilities, layout primitives, nav/footer components, palette names) should be smoke-checked against the docs site's rendered output before this lands. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Updates the docs site to @ably/ui@18.3.1 (crossing the 17→18 breaking change) and migrates the client-side session/API-key bootstrap away from the removed Redux-based APIs to the new Context + SWR (SessionDataProvider / useSessionData) pattern.
Changes:
- Bump
@ably/uifrom 17.14.0 to 18.3.1 (lockfile regenerated). - Replace Redux store/session/api-key bootstrapping with
SessionDataProvider+ a local asyncfetchApps()helper. - Remove obsolete Redux scaffolding/types and update mocks/tests to the new surface.
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Bumps @ably/ui dependency to 18.3.1. |
| yarn.lock | Updates lockfile entries for @ably/ui and transitive deps (Redux removed from @ably/ui deps). |
| gatsby-browser.tsx | Removes Redux store bootstrap; keeps setupObserver() and insights wiring. |
| src/components/GlobalLoading/GlobalLoading.tsx | Removes @ably/ui/core/scripts side-effect import tied to the old Redux setup. |
| src/contexts/user-context/wrap-with-provider.tsx | Wraps root with SessionDataProvider and switches user context session/apps sourcing to hooks + async fetch. |
| src/contexts/user-context/api-keys.ts | Adds docs-specific API key fetching helpers and endpoint constants (including global window.ably.docs.DOCS_API_KEY side effect). |
| mocks/handlers.js | Updates mock endpoint constant imports to the new module. |
| src/contexts/user-context.test.tsx | Updates the (skipped) test to use SessionDataProvider and the new apps data. |
| src/types/ably-ui-core-scripts/index.d.ts | Deletes now-invalid local typing shim for removed @ably/ui Redux exports. |
| src/redux/select-data.ts | Deletes unused Redux selector helper. |
| src/redux/fetch-and-add-to-store.ts | Deletes Redux data fetch/dispatch helper. |
| src/redux/api-key/remote-api-key-data.ts | Deletes Redux API-key retrieval logic now replaced by fetchApps(). |
| src/redux/api-key/index.ts | Deletes Redux API-key barrel. |
| src/redux/api-key/constants.ts | Deletes Redux endpoint/constants module (moved to api-keys.ts). |
| src/redux/api-key/api-key-reducer.ts | Deletes Redux reducer for API keys (store removed). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b9ce79b to
3b0fd5a
Compare
3b0fd5a to
5118095
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/contexts/user-context.ts (1)
56-59: ⚡ Quick winUse
interfaceforAppApiKeyto match the TS object-shape guideline.This keeps object-shape declarations consistent across the TS surface.
As per coding guidelines, "Use
interfacefor defining object shapes in TypeScript."Proposed change
-export type AppApiKey = { +export interface AppApiKey { name: string; whole_key: string; -}; +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/contexts/user-context.ts` around lines 56 - 59, The AppApiKey definition uses a type declaration instead of an interface, which is inconsistent with the project's TypeScript guideline that requires interfaces for object-shape definitions. Convert the AppApiKey type declaration to an interface by replacing the export type keyword with export interface and adjusting the syntax accordingly (remove the equals sign and semicolon, keeping the object shape properties intact). This ensures consistency across the TypeScript codebase for all object-shape declarations.Source: Coding guidelines
src/contexts/user-context/wrap-with-provider.tsx (1)
17-27: ⚡ Quick winUse
async/awaitinside the effect instead of chaining.then()/.catch().This keeps promise handling consistent with the TS guideline and improves readability in the cancellation flow.
As per coding guidelines, "Use async/await instead of .then() for promise handling in TypeScript."
Proposed change
useEffect(() => { let cancelled = false; - fetchApps() - .then((next) => { - if (!cancelled) { - setApps(next); - } - }) - .catch((e) => { - console.warn('Could not load api keys:', e); - }); + const loadApps = async () => { + try { + const next = await fetchApps(); + if (!cancelled) { + setApps(next); + } + } catch (e) { + console.warn('Could not load api keys:', e); + } + }; + void loadApps(); return () => { cancelled = true; }; }, []);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/contexts/user-context/wrap-with-provider.tsx` around lines 17 - 27, The useEffect hook in wrap-with-provider.tsx is using promise chaining with .then() and .catch() to handle the fetchApps() call. Refactor this to use async/await syntax instead. Create an async IIFE (immediately invoked function expression) inside the effect, use try/catch for error handling, preserve the cancelled flag check before calling setApps, and maintain the console.warn logging in the catch block with the error details.Source: Coding guidelines
src/contexts/user-context/api-keys.ts (1)
23-31: ⚡ Quick winPrefer
interfacefor payload object shapes in this TS module.As per coding guidelines, "Use
interfacefor defining object shapes in TypeScript."Proposed change
-type ApiKeyValue = { +interface ApiKeyValue { name: string; url: string; -}; +} -type ApiKeysPayload = { +interface ApiKeysPayload { data?: ApiKeyValue[]; error?: unknown; -}; +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/contexts/user-context/api-keys.ts` around lines 23 - 31, Replace the `type` keyword with `interface` keyword for the object shape definitions ApiKeyValue and ApiKeysPayload. Convert both type aliases to interfaces by changing `type ApiKeyValue = {` to `interface ApiKeyValue {` and `type ApiKeysPayload = {` to `interface ApiKeysPayload {`, removing the equals sign and adjusting the syntax accordingly. This aligns with the coding guidelines that prefer interfaces for defining object shapes in TypeScript.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/contexts/user-context.test.tsx`:
- Around line 17-34: The test for UserContextWrapper is currently skipped with
test.skip and needs to be re-enabled to provide coverage for this critical
integration path. Remove the .skip from the test function call to enable it, and
rename the test title from a generic component-based description to a
behavior-based description that clearly explains what is being tested (for
example, describing that it validates the session data and apps are properly
fetched and rendered). If MSW mocking prevents the integration test from
running, consider adding focused unit tests for the fetchApps function and its
branches instead of skipping the entire flow.
In `@src/contexts/user-context/api-keys.ts`:
- Around line 74-83: The fetchApps function currently awaits fetchDemoApp()
without error handling at the beginning, causing the entire function to fail if
that endpoint goes down and preventing the real API keys from being fetched.
Wrap the fetchDemoApp() call in its own try-catch block so that even if it
fails, the function continues to attempt fetching real API keys via the
fetchJson call to WEB_API_KEYS_DATA_ENDPOINT. If fetchDemoApp() fails, fall back
to an empty array or null value for the demoApp, then proceed with fetching and
returning the real apps so callers aren't left with no apps during a partial
outage.
---
Nitpick comments:
In `@src/contexts/user-context.ts`:
- Around line 56-59: The AppApiKey definition uses a type declaration instead of
an interface, which is inconsistent with the project's TypeScript guideline that
requires interfaces for object-shape definitions. Convert the AppApiKey type
declaration to an interface by replacing the export type keyword with export
interface and adjusting the syntax accordingly (remove the equals sign and
semicolon, keeping the object shape properties intact). This ensures consistency
across the TypeScript codebase for all object-shape declarations.
In `@src/contexts/user-context/api-keys.ts`:
- Around line 23-31: Replace the `type` keyword with `interface` keyword for the
object shape definitions ApiKeyValue and ApiKeysPayload. Convert both type
aliases to interfaces by changing `type ApiKeyValue = {` to `interface
ApiKeyValue {` and `type ApiKeysPayload = {` to `interface ApiKeysPayload {`,
removing the equals sign and adjusting the syntax accordingly. This aligns with
the coding guidelines that prefer interfaces for defining object shapes in
TypeScript.
In `@src/contexts/user-context/wrap-with-provider.tsx`:
- Around line 17-27: The useEffect hook in wrap-with-provider.tsx is using
promise chaining with .then() and .catch() to handle the fetchApps() call.
Refactor this to use async/await syntax instead. Create an async IIFE
(immediately invoked function expression) inside the effect, use try/catch for
error handling, preserve the cancelled flag check before calling setApps, and
maintain the console.warn logging in the catch block with the error details.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a9f4657a-2fd0-4f15-bc70-736f2f600de7
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (16)
gatsby-browser.tsxmocks/handlers.jspackage.jsonsrc/components/GlobalLoading/GlobalLoading.tsxsrc/contexts/user-context.test.tsxsrc/contexts/user-context.tssrc/contexts/user-context/api-keys.tssrc/contexts/user-context/wrap-with-provider.tsxsrc/redux/api-key/api-key-reducer.tssrc/redux/api-key/constants.tssrc/redux/api-key/index.tssrc/redux/api-key/remote-api-key-data.tssrc/redux/fetch-and-add-to-store.tssrc/redux/select-data.tssrc/types/ably-ui-core-scripts/index.d.tssrc/utilities/update-ably-connection-keys.test.ts
💤 Files with no reviewable changes (10)
- src/redux/api-key/constants.ts
- src/redux/api-key/api-key-reducer.ts
- gatsby-browser.tsx
- src/redux/select-data.ts
- src/types/ably-ui-core-scripts/index.d.ts
- src/redux/api-key/index.ts
- src/redux/fetch-and-add-to-store.ts
- src/redux/api-key/remote-api-key-data.ts
- src/components/GlobalLoading/GlobalLoading.tsx
- src/utilities/update-ably-connection-keys.test.ts
@ably/ui 18 removed the Redux scaffolding that this repo depended on (connectState, selectSessionData, fetchSessionData, getRemoteDataStore, createRemoteDataStore, reducerSessionData, reducerApiKeyData, reducerBlogPosts, reducerFlashes, attachStoreToWindow). Replaced with the Context + SWR pattern @ably/ui now exposes: - SessionDataProvider (SWR-backed) wraps the app at the root and fetches /api/me. UserContextWrapper reads from it via useSessionData and surfaces the result through the existing UserContext as sessionState, so downstream callers don't change. - API-key loading moved to a plain async fetch (src/contexts/user- context/api-keys.ts) driven by useEffect inside UserContextWrapper. This preserves the existing demo-key + real-key flow and the window.ably.docs.DOCS_API_KEY side effect for ad hoc scripts, while dropping the dispatch/reducer indirection. - gatsby-browser drops the createRemoteDataStore / attachStoreToWindow bootstrap; gatsby-ssr keeps the same default-export wiring, which now carries SessionDataProvider in addition to UserContext. - Side-effect import of @ably/ui/core/scripts in GlobalLoading is removed; that import existed to register the Redux store side effects, which no longer exist. - src/redux/ deleted in its entirety; constants live next to the fetcher that uses them. The stale type shim under src/types/ ably-ui-core-scripts is removed. - mocks/handlers.js, user-context.test.tsx point at the new module surface. The msw-blocked test stays skipped. No new dependencies; uses SWR transitively through @ably/ui. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5118095 to
8f978c8
Compare
|
Picked up the three nitpicks in CodeRabbit's review body alongside the actionable comments:
All in ~ 𝒞𝓁𝒶𝓊𝒹𝑒 |
Motivation
Catch docs up to the
@ably/uiline shipped on the other public surfaces. The headline reason for moving now is the Mixpanel EU-ingestion default that landed in 18.3.1: Mixpanel sunsets its US→EU forwarding bridge on 2026-07-01, and 18.3.1 defaultsmixpanel.inittoapi_host: https://api-eu.mixpanel.comso docs routes directly to the EU project instead of relying on the soon-to-be-removed forwarder.This crosses a major version boundary (17 → 18), and the major dropped the Redux scaffolding the package used to expose. This PR does both the version bump and the migration off those removed APIs.
What changed
Commit 1 —
chore: bump @ably/ui from 17.14.0 to 18.3.1package.json:@ably/ui17.14.0 → 18.3.1yarn.lockregenerated viayarn installCommit 2 —
refactor: migrate session/api-key bootstrap off @ably/ui Redux APIs@ably/ui 18 removed
connectState/selectSessionData/fetchSessionData/getRemoteDataStore/createRemoteDataStore/reducerSessionData/reducerApiKeyData/reducerBlogPosts/reducerFlashes/attachStoreToWindowand replaced them with a Context + SWR pattern (SessionDataProvider,useSessionData,FlashProvider,useFlashContext).This repo's bootstrap is ported across:
src/contexts/user-context/wrap-with-provider.tsxnow reads session data fromuseSessionData()(provided bySessionDataProviderat the root) and runs the api-key fetch through a plain async function insideuseEffect. The component still publishes the sameUserContextshape (sessionState,apps), so every downstream consumer keeps working unchanged.src/contexts/user-context/api-keys.ts(new) holds the WEB_API endpoint constants and thefetchApps()async helper. The demo-key + real-key flow is preserved, including thewindow.ably.docs.DOCS_API_KEYside effect needed by ad hoc scripts.gatsby-browser.tsxloses thecreateRemoteDataStore/attachStoreToWindowbootstrap;onClientEntryis now justsetupObserver().gatsby-ssr.tsxkeeps the same default-export wiring —wrap-with-provider.tsx's default now carriesSessionDataProviderin addition toUserContextWrapper, so SSR inherits the new provider without any SSR-side edits.src/components/GlobalLoading/GlobalLoading.tsxdropsimport '@ably/ui/core/scripts'; that side-effect import existed to register Redux store side effects that no longer exist.src/redux/deleted in its entirety. The constants that used to live there (WEB_API_USER_DATA_ENDPOINT,WEB_API_KEYS_DATA_ENDPOINT,WEB_API_TEMP_KEY_ENDPOINT) now live next to the fetcher that consumes them, insrc/contexts/user-context/api-keys.ts.src/types/ably-ui-core-scripts/index.d.tsdeleted — the local TS shim describing the removed Redux types was lying about the module's surface.mocks/handlers.js,src/contexts/user-context.test.tsxpoint at the new module surface. The msw-blocked test stays skipped (TypeError: The "eventTargets" argument must be an instance of EventEmitter or EventTarget mswjs/msw#1785).No new dependencies are added. SWR is transitively present via
@ably/ui.Things to be cautious about in review
SessionDataProvidertypes vs docsSessionState.useSessionData()is typed by@ably/uias{ user?: { firstName?, lastName?, email? } } | null, but at runtime the/api/meendpoint returns the rich shape docs has always read. The wrapper casts viaas unknown as SessionStateto keep the existing UserContext consumers happy. If/api/meever stops returning a superset of what the consumers expect, the cast will hide it — worth keeping in mind.window.ably.docs.DOCS_API_KEYside effect. Preserved verbatim from the previous code path so ad hoc scripts that read this global don't break. The comment that flagged this as "remove when ad hoc scripts are removed" is also preserved.Flashis no longer wired. The old bootstrap registeredreducerFlashesinto the Redux store, but no Flash component is rendered anywhere in this repo.FlashProvideris intentionally not added. The@ably/ui/core/Flash/component.cssimport insrc/styles/global.cssstays — it's pure CSS and harmless.@ably/ui18 peers on React 18; this repo already uses React 18, so no change needed there.yarn tsc --noEmiton this branch produces the same set of pre-existing errors asorigin/main— none of the new files contribute additional errors. Worth confirming the repo's CI doesn't block ontsc(it currently doesn't appear to).yarn lint,yarn test(303 pass, 1 skipped — the pre-existing msw-blocked test), andyarn tsc --noEmit(no new errors from the changed files) all green locally.yarn buildnot run locally due to its cost; CI on the PR will exercise it.What 17.14 → 18.3.1 brings (consumer-facing)
SessionDataProvider+FlashProviderreplace the old store. (Breaking — migrated in this PR.)react/no-dangerlint rule; PostHog session-recording plumbing removed from insights; vitest 4, posthog-js bumps.mixpanel.initdefaults to EUapi_host; tsconfig lib bumped to ES2020; CI runspnpm buildon PRs.How do you manually test this?
mixpanel.comrequests targetapi-eu.mixpanel.com(notapi-js.mixpanel.com)./api/meand/api/api_keys(proxied throughGATSBY_ABLY_MAIN_WEBSITE); confirm the calls fire from the new provider/effect rather than the old Redux dispatch.yarn build— must complete with no missing-export errors from@ably/ui/core/scripts.Follow-ups (after this lands)
api-eu.mixpanel.comfor incoming docs events.yarn build(or another equivalent type-checking gate) isn't currently in CI for this repo, consider adding it — pure runtime tests won't catch missing-export breakages of this kind.window.ably.docs.DOCS_API_KEYad hoc script support can be deleted as soon as those external scripts are gone (carried over comment from the previous implementation).🤖 Generated with Claude Code
Summary by CodeRabbit
@ably/uilibrary from version 17.14.0 to 18.3.1, providing enhanced functionality and improved stability