Repository navigation
feat: add the frontend-base route tree with stub pages, roles and provides - #2221
Open
brian-smith-tcril wants to merge 1 commit into
Open
brian-smith-tcril wants to merge 1 commit into
brian-smith-tcril wants to merge 1 commit into
Conversation
brian-smith-tcril
added a commit
to brian-smith-tcril/bairf
that referenced
this pull request
Oct 22, 2026
…body drafted Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## bsmith/test-console-noise #2221 +/- ##
=============================================================
+ Coverage 96.16% 96.17% +0.01%
=============================================================
Files 374 377 +3
Lines 6020 6046 +26
Branches 1479 1483 +4
=============================================================
+ Hits 5789 5815 +26
Misses 222 222
Partials 9 9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…vides One root route at `learning` with the Learning role, rendering Main; course layout routes at `course/:courseId` and `preview/course/:courseId` carrying the `learningCourse` role, which `provides` lists for the shell's course bar and masquerade bar so both mount only where a course id exists; a child route for every URL Learning serves today, each rendering a stub that names its page as not yet ported; a role on the four destination tabs. Nothing fetches and no provider mounts. Route tests render the exported array through createMemoryRouter at every path. jsdom has no Fetch API and the data router builds a Request for a route with `lazy`, so a jest environment forwards Node's fetch classes into the sandbox, as react-router's own suite does from undici. Part of #2188. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
brian-smith-tcril
force-pushed
the
bsmith/frame-routes
branch
from
October 22, 2026 18:54
c020ad5 to
d638d38
Compare
brian-smith-tcril
marked this pull request as ready for review
October 22, 2026 19:05
This was referenced Oct 23, 2026
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.
Summary
Learning's route tree now exists in the frontend-base package and the shell knows about it: one root route at
learning, course layout routes atcourse/:courseIdandpreview/course/:courseIdcarrying alearningCourserole, a child route for every URL Learning serves today, each rendering a stub that names its page as not yet ported, a role on the four destination tabs, andprovideslistinglearningCourseso the shell's course bar lists Learning's tabs and switches between them client-side, with the masquerade bar on course routes only. Nothing fetches, no provider mounts, no page is ported. No user-facing change onmaster.Part of #1905. Resolves #2188. Stacked above #2219.
What changed
src/routes.tsx: the rootlearningroute (rolelearning, lazy-loadingMain); two course layout routes (bare outlets, rolelearningCourse) holding one named array of explicit route objects per course page, with the bare course path as the course layout's index route; the three non-course pages under the root. Ids per page, roles on the outline tab, dates, progress and courseware.src/constants.ts:learningCourseand the four page role ids beside the root role.src/provides.ts,src/app.ts:learningCourseunder both course-bar ids, as instructor dashboard declares its own course-scoped role.src/StubPage.tsx: one line naming the page and a list of the params the route captured.jsdom-environment-with-node-fetch.mjs, withjest.config.jspointing at it andtsconfig.jsonincluding it: forwards Node's Fetch classes into jsdom, which has none, so route tests can start react-router's data router.src/tests/render.tsx(memory router plus intl),src/routes.test.tsx(one block per page),src/app.test.ts(role declarations,provides, what each role resolves to);tsconfig.build.jsonexcludessrc/**/tests/**/*so helpers never ship.Testing
In
frontend-base/: lint, 29 tests at 100% lines with no console output,buildandbuild:cigreen. The rootmake validatedoes not see the package. Manual: the stubs, the course bar and the masquerade bar on the package's dev site against tutor dev, recorded below.Decisions
What was decided while building this layer; the route table, the roles and
providesare in #2188.Full decision log
Decisions — route tree with stub pages, roles and provides (#2188)
Decisions made while implementing. The route table, roles,
providesandthe test approach are in #2188's plan and are not repeated.
Route tests render the real array through a data router, and the
jest environment supplies the Fetch globals from Node. The first run
of
routes.test.tsxfailed withReferenceError: Request is not defined: react-router's data routers (createMemoryRouter, the testtwin of the shell's
createBrowserRouter) build aRequestatinitialization for any route with
lazyor a loader, and jest's jsdomenvironment has no Fetch API. The facts behind the fix, as read on
2026-10-22:
jsdom test environment"): "
fetchis only available in using thenode environment atm. See Implement the Fetch API jsdom/jsdom#1724." That jsdom issue,
"Implement the Fetch API", is open (jsdom 30.1.3 is current); even
the newest
jest-environment-jsdom(30.5.2) has noRequest. Thepackage is on jest 29 with jsdom 20.
packages/react-router/__tests__/setup.tson
main(last changed 2026-05-28, react-router 8.5.0, underjest-environment-jsdom), assignsglobalThis.fetch,Request,ResponseandHeadersfromundiciwhen they are missing;Drop support for Node 18 and installGlobals remix-run/react-router#12171 dropped the library's
installGlobalsbecause "
fetchis stable in Node 20", telling users to "write yourown
installGlobalsin userland".whatwg-fetchwas rejected: [Bug]:useActionDatareturns rawResponsewhen using polyfilledfetchremix-run/react-router#9722 shows it isthe polyfill CRA used, that it "does not implement
.body", and themaintainer declined to "support a non-spec-compliant
fetchpolyfill just for testing purposes". One openedx repository lists it
(frontend-app-ora-grading, a legacy MFE) with no import found.
testing page is a pointer to the framework guide, which names no
setup), and steer route-level tests toward end-to-end runs. The
redirect layers (Port the discussion tab to frontend-base as a redirect #2204, Port the course access redirects to frontend-base #2205) port as loaders returning
redirect(),whose value is a
Response, so the globals are needed for their unittests regardless; Brian took that as the reason to set route testing
up now.
The mechanism is a custom jest environment,
jsdom-environment-with-node-fetch.mjs,extending
jest-environment-jsdomand forwarding Node's ownfetch,Headers,Request,Response,AbortControllerandAbortSignalinto the sandbox (the last two because the router passes an
AbortSignalto theRequest, and Node'sRequestrejects jsdom's:"Expected signal to be an instance of AbortSignal"). Node's classes
rather than the
undicipackage: asetupFilesscript runs inside thesandbox and cannot reach Node's globals (Jest globals differ from Node globals jestjs/jest#2549), which is
why react-router imports
undici; an environment runs outside it andcan hand over the classes Node already ships, the same implementation,
with no dependency and the version pinned by
.nvmrc. The pattern isjest-fixed-jsdom's (mswjs), whose export-conditions change is nottaken. The file is an ES module,
jsdom-environment-with-node-fetch.mjs(class
JSDOMEnvironmentWithNodeFetch, named for what it is ratherthan for the app), which jest 29 loads as a test environment (checked
by running the suite), so it needs no
requireand therefore noexception in
eslint.config.js,which stays the template's shape like every converted app's; a
CommonJS version would have needed an override, since the base lint
config exempts only
babel.config.js,jest.config.jsandeslint.config.jsfrom therequirerule.jest.config.jspointstestEnvironmentat the file; the file's one comment says it can beremoved if the jsdom issue is resolved (the rest of this entry is the
rationale and stays here);
tsconfig.jsonincludes it beside theother config files, which is what lets the TypeScript project service
parse it for lint.
No unmatched-path test. Rendering the array alone at a path no
child matches reaches react-router's default error boundary, which
logs to
console.errorand fails under the console hook; the shell'serrorElementis what handles it at runtime. The not-found behavior isthe shell's and is checked on the dev site.
Four page roles, named with the glossary; route ids share the page
key. The plan listed six roles. Live and course exit lost theirs,
by the rule Add frontend-base route tree with stub pages, roles and provides #2188's plan states: a role is a promise that
another app may resolve it to link there, live is reached only from
the course bar (which navigates by the tab URLs
course_metadatasupplies, not by role) and course exit only from inside courseware.
The four that stay,
courseOutline,courseDates,courseProgressand
courseware, each have a plausible outside consumer and are each acourse-bar tab. The landing tab's role is
courseOutlinerather thanthe config review's
courseHome: the repo glossary's "course home" isthe whole tab-page side, the effort glossary's name for the page is
"outline tab", and the shell's course-bar slug is
outline; thecollision with the navigation sidebar that the glossary guards against
is not a risk inside a role id. The
courseprefix on dates andprogress is disambiguation, not app naming (bare
datessays nothingabout what they are dates of), on the pattern of catalog's
courseAbout;coursewareis the platform's own word and stays oneword like
gradebook. Route ids areorg.openedx.frontend.route.learning.<page>with the page key on thefirst path of a page, and the key matches the role's noun where a role
exists (
outline,dates,progress,courseware); the stub printsthe same key, so the dev site and the tests name a page one way.
A role declared on several paths resolves to the first one declared
(frontend-base's
getUrlByRouteRolewalks the tree in order), so thebase path is listed first for progress and courseware, and a
role resolutionblock has one test per role naming the URL itresolves to and, for the two multi-path roles, what it must not be (a
sequence, unit or preview path; another user's progress): the order is
what every other app's link depends on, nothing else in the suite
would notice a reorder, and one case per role reads as the table of
link targets. The route tests
assert declarations (
declaredRolesByChildPath), not which roles areactive at a URL; the runtime merges ancestors' roles, which is
frontend-base's to test.
Route objects written out, one named array per page. A first cut
built the sixteen child routes through a helper with conditional
spreads; it hid what each route was. The file now has one
RoleRouteObject[]per page (outlineRoutes,coursewareRoutes, ...)with every object explicit, spread into the root's children in table
order. A port replaces its page's array with real routes, so the
explicit shape is the one that lasts; the repetition across a page's
paths is the honest shape of "one route object per path".
Test helpers live in
src/tests/, excluded from the build. Thehelper that renders the route array under a memory router and the intl
provider started in a new
src/testing/directory.masterkeeps itshelpers in
src/tests/(CourseQueryGate.tsx,MountCourseQueryHooks.tsx,SidebarState.tsx), as do account and profile, and the ports bringthose files along, so the helper moved there. The template's
tsconfig.build.jsonexcludes only test files, mocks and the setupfile, which would have shipped the directory in
dist/(instructordashboard's
testUtils.tsxis in its published build); account'ssrc/**/tests/**/*exclusion is added and a build confirmeddist/has no
tests/.Route tests are one
describeper page. The first cut was oneit.eachtable of seventeen rows, hard to read and wrong the moment apage is real. Each page now has its own block and one test per path,
through
renderAt(path)andexpectStub(page, params), where theparams a case names are the ones its path captures;
StubPageprintsone list item per param for that. A page's block changes with its port
instead of being deleted, so the file keeps saying that every URL
reaches its page through the real array. The test course id is
course-v1:OpenedX+DemoX+DemoCourse, the current demo course.The course bar and masquerade roles come from course layout routes,
not the root. The manual pass found
GET /courses//masquerade404four times on the goal-unsubscribe page: the shell mounts
MasqueradeBarwherever a role listed under the masquerade id isactive, the root
learningrole was that role and is active on everyLearning route, and
MasqueradeBarhandsuseParams().courseId,''on a non-course page, to a query with noenabledguard (itssibling, the course bar's
course_metadataquery, hasenabled: !!courseId). The bar renders nothing because the 404 reads as denied;the request and the console noise remain, refetched on each window
focus. The deeper point, from Brian: a course-bar role declares "this
route is a course page" (frontend-base ADR 0013's example and
instructor dashboard's role both sit on a
:courseIdroute), andlearningon the root said that of three pages with no course. Sothe tree gained two layout routes,
course/:courseIdandpreview/course/:courseId, each anOutletcarrying a new role,org.openedx.frontend.role.learningCourse, andprovideslists thatrole under both ids.
learningstays on the root meaning "inside theLearning app", unprovided; the page roles are unchanged; the three
non-course pages sit directly under the root.
learningCourseresolves to
/learning/course/:courseId, which prefix-matches everytab URL for client-side navigation, and the masquerade bar now mounts
only where a course exists. Two notes for frontend-base, not filed:
the unguarded masquerade query, and that a role on an index route
resolves with a trailing slash (
findRoleInRoutesjoins an emptysegment), which is why
coursewareresolves to/learning/course/:courseId/; harmless for navigation.Manual testing
Manual testing — route tree with stub pages, roles and provides (#2188)
This layer claims no page content: every route renders a stub naming
its page, and what is verified by hand is the routing, the roles and the
provideswiring as the shell sees them.Environment
cd frontend-base && nvm use && npm ci && npm run devon the layer branch, served at
http://apps.local.openedx.io:2000/learning/,with tutor dev's
mfeservice stopped so the port is free.course-v1:OpenedX+DemoX+DemoCourse, theLMS's learning URL pointing at
apps.local.openedx.io:2000/learningsothe tabs
course_metadatalists resolve to this site, signed in as astaff user (the masquerade bar is staff-only).
Verify by hand
/learning/course/<courseId>/home: the shell header and footer, thecourse bar listing the course's tabs, and the body "outline: not yet
ported" with
courseId=<courseId>under it.the body changes to that tab's stub with no document request in the
network panel and the console keeping its earlier lines. The
discussion tab, whose LMS URL points at the discussions app, may be
a full-page link; note which it is.
renders on the course home stub and on a courseware stub.
/learning/course/<courseId>/<seq>/<unit>shows "courseware: not yet ported" with
sequenceIdandunitIdlisted;
/learning/preview/course/<courseId>/<seq>likewise./learning/course/<courseId>/progress/42shows the progress stub with
targetUserId=42./learning/goal-unsubscribe/tok,/learning/redirect/dashboardand/learning/preferences-unsubscribe/tokeach show their stub, with the shell header and footer (no course
bar, since no course route is active).
/learning/nopeshows the shell's not-foundpage, with header and footer this time, since the root route matched.
pages the network panel shows no request to
/courses//masquerade(the first pass found four; see results).
Left to the automated suite
src/routes.test.tsx, one block per page.provides, and what each role resolves to:src/app.test.ts.Results
First pass, 2026-10-22, Brian, on the tree with the root
learningroleprovided (
892f928b). Course home rendered its stub under the shell withthe course bar (box 1 passed). The discussions tab was absent from the bar:
course_metadatalists it only when discussions are enabled on the course,and the shell renders exactly that list; enabling discussions in Studio's
Pages & Resources brings it back. Non-course pages showed the shell header
and footer with no course bar or masquerade bar, as expected. Console on
the goal-unsubscribe page:
GET http://local.openedx.io:8000/courses//masquerade404, four times (refetched on window focus), from the shell's masquerade
bar mounting on a non-course route under the root role and querying with
an empty course id (decision 7); the React Router future flag warning
(openedx/frontend-base#325);
Nav: Support for defaultProps will be removed, react-bootstrap'sNavin the shell header;UNSAFE_componentWillMount ... SideEffect(NullComponent), react-helmet under StrictMode; webpack-dev-serverwebsocket and back-forward-cache lines. Nothing from the layer's own code.
The tree was restructured in response (course layout routes carrying
learningCourse); every box is re-checked on the restructured tree.Second pass, 2026-10-22, Brian, on the restructured tree (
d638d386,local branch before push). All nine boxes passed, including the new one:
no request to
/courses//masqueradeon the non-course pages, since themasquerade bar no longer mounts there.
🤖 Generated with Claude Code