YPE-1565: Show error when YouVersionProvider has no appKey#264
YPE-1565: Show error when YouVersionProvider has no appKey#264cameronapak wants to merge 9 commits into
Conversation
Render a styled "Missing app key" message instead of a blank page; hooks provider throws for hooks-only consumers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 2ddd583 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
role="alert" already implies aria-live="assertive"; the explicit polite value downgraded announcement urgency for a config error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Remove redundant `missingAppKey*` translation keys - Reuse generic `errorHeading` and `invalidAppKeyError` strings - Add a `console.error` in `YouVersionProvider` for developers
The previous commit reused errorHeading/invalidAppKeyError and moved the actionable developer guidance to console.error. Update the assertions to match: expect "Error" instead of "Missing app key", and verify the console.error guidance fires. Also refresh the guard comment to reflect that the panel is intentionally generic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@Dustin-Kelley can you review this? I'm no longer adding new strings to this and re-using old ones P.S. I'll learn the new strings process and share that with you soon |
Splits YouVersionProvider so the missing-appKey throw lives in a thin wrapper and the hook-bearing implementation runs in an inner component. The inner component is never entered with an invalid key, so a mounted provider transitioning between valid and empty appKey can't produce an inconsistent hook count. Addresses Greptile P1 review feedback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| theme={theme} | ||
| apiHost={apiHost} | ||
| appKey={appKey} | ||
| appKey={appKey ?? ''} |
There was a problem hiding this comment.
nitpick(non-blocking): I don't know too much about this codebase yet so I could be totally wrong. If it's possible I usually like to guard against the empty appKey before it gets passed to this provider so you can always expect it to be there within the provider. Not sure if that's possible in this case though. Either way it gets the job done!
Empty/missing
appKeyrendered a blank page. Now: UI provider shows a styled "Missing app key" message; hooks provider throws.MissingAppKeypanel + Storybook storyYPE-1565
🤖 Generated with Claude Code
Greptile Summary
This PR surfaces a clear error when
YouVersionProviderreceives a missing or emptyappKey, replacing the previous silent blank-page behaviour. The UI package renders a styledMissingAppKeypanel while the hooks-only package throws a descriptiveErrorat the outermost wrapper so the hook-bearing inner component is never entered with an invalid key.YouVersionProviderinto a guard wrapper (throws on invalid key) andYouVersionProviderInner(owns all hooks), correctly sidestepping the Rules of Hooks concern from the previous review thread.resolveThemehelper and guardsappKeybefore delegating to the base provider;MissingAppKeyuses existing i18n keys and arole="alert"panel without the conflictingaria-liveattribute flagged earlier.undefined, empty string, and whitespace-only cases for both packages; a Storybookplaystory validates the rendered panel end-to-end.Confidence Score: 5/5
Safe to merge — the change is additive and has no effect on consumers with a valid appKey.
The wrapper + inner split correctly avoids hook-order issues, tests cover all three invalid-key variants in both packages, and the changeset is correctly scoped as minor for both packages.
packages/ui/src/components/YouVersionProvider.tsx — the console.error fires in the render function body rather than in an effect, which causes duplicate log lines under React Strict Mode.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Consumer mounts YouVersionProvider] --> B{appKey valid?} subgraph UI Package B -- "no (empty/whitespace/undefined)" --> C[console.error to developer] C --> D[Render MissingAppKey panel + YvStyles] B -- yes --> E[Render BaseYouVersionProvider + YvStyles + children] end subgraph Hooks Package - BaseYouVersionProvider E --> F{appKey valid?} F -- no --> G[throw Error - non-empty appKey required] F -- yes --> H[YouVersionProviderInner - useResolvedTheme / useMemo / useEffect / YouVersionContext.Provider] end G --> I[Error boundary or uncaught crash] H --> J[Children rendered with full context] D --> K[Children suppressed - error panel only]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[Consumer mounts YouVersionProvider] --> B{appKey valid?} subgraph UI Package B -- "no (empty/whitespace/undefined)" --> C[console.error to developer] C --> D[Render MissingAppKey panel + YvStyles] B -- yes --> E[Render BaseYouVersionProvider + YvStyles + children] end subgraph Hooks Package - BaseYouVersionProvider E --> F{appKey valid?} F -- no --> G[throw Error - non-empty appKey required] F -- yes --> H[YouVersionProviderInner - useResolvedTheme / useMemo / useEffect / YouVersionContext.Provider] end G --> I[Error boundary or uncaught crash] H --> J[Children rendered with full context] D --> K[Children suppressed - error panel only]Reviews (8): Last reviewed commit: "refactor(hooks): move appKey guard to wr..." | Re-trigger Greptile