perf: cut published package and partner-bundle bytes (YPE-5528) - #382
perf: cut published package and partner-bundle bytes (YPE-5528)#382cameronapak wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: 0803452 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 |
dc137d2 to
f6bf5c9
Compare
| return ensureLocale(detected).then(() => { | ||
| if (i18n.language === detected) { | ||
| return detected; | ||
| } | ||
| return i18n.changeLanguage(detected).then(() => detected); | ||
| }); |
There was a problem hiding this comment.
If two non-English locale requests overlap, each continuation calls changeLanguage after its own import resolves without checking which locale was requested most recently. A slower older request can therefore overwrite the newer locale and leave SDK copy displayed in the wrong language.
Knowledge Base Used: React UI component library
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/i18n/index.ts
Line: 69-74
Comment:
**Stale locale requests win**
If two non-English locale requests overlap, each continuation calls `changeLanguage` after its own import resolves without checking which locale was requested most recently. A slower older request can therefore overwrite the newer locale and leave SDK copy displayed in the wrong language.
**Knowledge Base Used:** [React UI component library](https://app.greptile.com/youversion/-/custom-context/knowledge-base/youversion/platform-sdk-react/-/docs/react-ui.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| subscribeSdkLanguage((languageTag) => { | ||
| void syncSdkLanguage(languageTag); | ||
| }); | ||
| const pendingLocale = getRequestedSdkLanguage(); | ||
| if (pendingLocale.requested) { | ||
| void syncSdkLanguage(pendingLocale.languageTag); |
There was a problem hiding this comment.
If a non-English locale chunk cannot be fetched, ensureLocale rejects and these production call sites discard the promise without a rejection handler. This emits an unhandled promise rejection while leaving the requested language inactive, and runtimes configured to treat unhandled rejections as fatal can terminate the affected execution context.
Knowledge Base Used: React UI component library
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/i18n/index.ts
Line: 105-110
Comment:
**Locale load rejections escape**
If a non-English locale chunk cannot be fetched, `ensureLocale` rejects and these production call sites discard the promise without a rejection handler. This emits an unhandled promise rejection while leaving the requested language inactive, and runtimes configured to treat unhandled rejections as fatal can terminate the affected execution context.
**Knowledge Base Used:** [React UI component library](https://app.greptile.com/youversion/-/custom-context/knowledge-base/youversion/platform-sdk-react/-/docs/react-ui.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Stack 2/3 of #382. Hooks-only changes: deep imports of YouVersionContext, client-override fallback shape per hook, and query-key changes.
… (YPE-5528) Stack 3/3 — UI dist split, i18n lazy locales, Storybook sheet loading, and the CI gates (tree-shaking UI row, size budgets, spec sentinels).
e82174e to
0803452
Compare
Summary
distso a named import from the package root can drop unused Bible UI. No new public subpaths.BibleClient/LanguagesClientoff narrow hook graphs. Host client overrides still win.YPE-5528. Parent YPE-1193.
Size saved per import path
Minify + brotli, esbuild, no splitting. React, react-dom, jsx-runtime, react-query, and jsdom are external. Same ruler as
docs/ui-import-size-research.md.On
main(5ddc2e0), any one named UI import measured like Provider: 196.1 kB. The full barrel was 203.6 kB. The gap was 42 kB raw.@youversion/platform-react-uimain{ YouVersionProvider }{ BibleReader }{ BibleCard }{ Separator }import *full barrelNamed import from the root matches the component entry file. Do not add
./bible-reader.pnpm sizeon this branch (same no-split gate):mainbudget{ ApiClient }{ useChapter }{ YouVersionProvider }BibleReader stays large because of i18next, the full utility sheet, and passage/jsdom. Replacing i18next was considered and skipped. Provider already avoids that runtime.
Test plan
CI=true pnpm sizestays under the new budgetspnpm check:tree-shaking(Provider absent Bible reader/picker sentinels;useChapterabsentBibleClient/ jsdom)examples/vite-react: Provider-only page, then BibleReader + pickers, then Sign inrequire('@youversion/platform-react-ui')still resolveslocaleon Provider still translates missing-app-key copy without loading i18nextstyles.cssstill includes reader typographyGreptile Summary
The PR restructures the UI package for smaller consumer bundles while preserving its root public API.
Confidence Score: 3/5
The PR is not yet safe to merge because locale requests can still resolve out of order and failed locale chunks can still produce unhandled promise rejections.
The current lazy-loading path allows an older locale import to apply after a newer request, while dynamic-import failures propagate into multiple fire-and-forget callers without rejection handling.
Files Needing Attention: packages/ui/src/i18n/index.ts
Important Files Changed
Sequence Diagram
Reviews (2): Last reviewed commit: "chore(changeset): scope the UI split cha..." | Re-trigger Greptile
Context used: