feat!: bundle only real defaults in app config - #1688
Merged
arbrandes merged 3 commits intoAug 29, 2026
Conversation
Every key an app declares in its own `config` is a key `commonAppConfig` can never supply, since app config is the highest-precedence layer. Authn declared twenty-nine, so values an operator set platform-wide through `MFE_CONFIG` silently lost to bundled defaults. Three survive as genuine defaults and move to `App.defaultConfig` (frontend-base ADR 0017, added by openedx/frontend-base#298), which resolves below `commonAppConfig`: `DISABLE_ENTERPRISE_LOGIN`, whose absence would quietly re-enable the enterprise flow, and `LOGO_URL` and `LOGO_WHITE_URL`, which carry the same default frontend-base ships for the shell header. Two of the rest were read nowhere at all, `FAVICON_URL` and `LOGO_TRADEMARK_URL`. Fifteen were no-ops against reads that already guard for absence. The remaining nine were placeholders whose reads did not guard, and those reads now do: `SEARCH_CATALOG_URL` was a latent `TypeError` in the embedded welcome-page flow; `INFO_EMAIL` produced `mailto:undefined`; the activation and password-reset support links rendered dead anchors; and the four `BANNER_IMAGE_*` keys asked the browser for `/undefined`. `MARKETING_SITE_BASE_URL` wrapped the logo in a link at ten near-identical sites and defaulted to a devstack host. Those collapse into one `BrandLogo`, which renders the logo unlinked when no marketing site is configured, rather than pointing it at the app's own origin. The peer dependency moves to `@openedx/frontend-base` 2.0.0-alpha.4, the first release carrying `App.defaultConfig`. Three test defects surfaced while auditing and are fixed here, since the audit is what makes the boolean contract real: `AccountActivationMessage.test.jsx` called `mergeAppConfig` without its `appId`, so its overrides never landed and the confirmation suite asserted the activation copy; boolean flags were overridden with env-era strings that strict `=== false` reads can never honour; and `SHOW_CONFIGURABLE_EDX_FIELDS` was set in four tests but read nowhere. Part of openedx#1687. Co-Authored-By: Claude <noreply@anthropic.com>
The shared ESLint config in frontend-base 2.0 requires semicolons rather than commas between TypeScript type members. Mechanical `npm run lint:fix`. Part of openedx#1687. Co-Authored-By: Claude <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1688 +/- ##
==========================================
+ Coverage 92.03% 93.64% +1.60%
==========================================
Files 93 94 +1
Lines 2047 2076 +29
Branches 588 603 +15
==========================================
+ Hits 1884 1944 +60
+ Misses 160 129 -31
Partials 3 3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
`BaseContainer.test.jsx` passed `wrappingComponent` to React Testing Library, an enzyme option RTL ignores, so no breakpoint ever matched and neither layout rendered. Its two assertions were `toBeDefined()` on a `querySelector` that returned `null`, which passes for anything. With a real wrapper the image and welcome-page layouts are exercised for the first time, and both go from a quarter covered to whole. The remaining new tests cover the guards the audit added: banner backgrounds with and without an image configured, the activation and password-reset support links in both their linked and plain-text forms, the forgot-password support address, and `ProgressiveProfilingPageModal.handleSubmit`, which was untested altogether and is where the `SEARCH_CATALOG_URL` crash landed. Coverage goes from 92.00% to 93.54%. Part of openedx#1687. Co-Authored-By: Claude <noreply@anthropic.com>
arbrandes
added a commit
that referenced
this pull request
Aug 29, 2026
Re-releases #1688, whose `feat!` header alone triggered no release. BREAKING CHANGE: App config no longer bundles twenty-nine keys, so operator values from MFE_CONFIG reach the app. Requires frontend-base 2.0.0-alpha.4. Co-Authored-By: Claude <noreply@anthropic.com>
|
🎉 This PR is included in version 2.0.0-alpha.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Every key an app declares in its own
configis a keycommonAppConfigcan never supply, since app config is the highest-precedence layer. Authn declared twenty-nine, so values an operator set platform-wide throughMFE_CONFIGsilently lost to bundled defaults. Three are genuine defaults and move toApp.defaultConfig(frontend-base ADR 0017), which resolves belowcommonAppConfig. Two others were read nowhere, fifteen were no-ops against reads that already guard for absence, and the nine placeholders whose reads did not guard now do.The peer dependency moves to
@openedx/frontend-base2.0.0-alpha.4, the first release carryingApp.defaultConfig. The second commit is a mechanicallint:fixfor the semicolon member-delimiter rule 2.0 enables, and the third covers the new guards.Part of #1687. Follows openedx/frontend-app-catalog#156.
LLM usage notice
Built with assistance from Claude.