[ZEPPELIN-6428] Add reusable React mount infrastructure and migrate paragraph footer behind a flag#5266
Open
tbonelee wants to merge 24 commits into
Open
Conversation
� Conflicts: � zeppelin-web-angular/projects/zeppelin-react/package-lock.json � zeppelin-web-angular/projects/zeppelin-react/package.json
Reads the `reactFooter` query param in NotebookComponent and passes a `useReactFooter` flag down to each paragraph. The paragraph swaps the Angular footer for a React micro-frontend mount when the flag is true, with an `onReactFooterError` fallback that reverts to the Angular footer on any React render failure.
ReactRemoteLoaderService's `declare global { Window.reactApp?: RemoteContainer }`
now types `container.get(key)` as returning `Promise<() => unknown>` by
default, breaking the destructure of `mount` in the PublishedParagraph
pilot. Pin the generic to the expected mount shape.
PublishedParagraph will be migrated onto the new directive in a follow-up
PR; this is the minimum change to keep the existing integration compiling.
… callbacks Codex review caught a real bug: React error boundaries fire componentDidCatch outside the Angular zone (the directive mounts via ngZone.runOutsideAngular), so calling props.onError directly leaves the host's cdr.markForCheck() with nothing to flush. The fallback UI would only appear on the next unrelated change-detection tick. Wrap onError dispatch in ngZone.run so the host can safely mutate state and trigger CD.
The getter returned a fresh object literal on every call, so each change-detection pass handed ReactMountDirective a new reference and triggered handle.update() even when nothing changed. The implementation plan (Task 11 Step 3) and the zeppelin-react README both promise stable identity when the underlying values are unchanged. Cache the last emitted props object and return it as long as a shallow comparison finds no value change, so ngOnChanges fires only on real updates. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Assisted-By: Claude <noreply@anthropic.com>
The frontend README requires export folders to carry public-api.ts (actual exports) plus index.ts that only re-exports ./public-api, the pattern resize-handle and shortcut already follow. react-mount shipped with a bare index.ts and was missing from the share/public-api.ts barrel, so it wasn't reachable via the @zeppelin/share mapped path. Add react-mount/public-api.ts, reduce index.ts to the re-export, and register react-mount in the share barrel. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Assisted-By: Claude <noreply@anthropic.com>
…ar footer The fallback chain (loader rejection -> directive reportError -> host reactFooterFailed -> Angular footer re-render) had no E2E coverage; the existing tests only exercised the happy path and destroy-while-loading. Abort every remoteEntry.js request and assert the Angular footer renders while the React footer is absent. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Assisted-By: Claude <noreply@anthropic.com>
zeppelin-react had no unit-test runner, so the error boundary and the
Module Federation mount contract were only exercised indirectly through
Playwright. Add a self-contained vitest + jsdom + Testing Library setup
(vitest 3.x; vitest 4 conflicts with the pinned @types/node@18) and
cover the behavior the Phase 2 design specified:
- ReactErrorBoundary: renders children when healthy; renders nothing
and reports the error exactly once when a child throws; swallows
onError callback failures; tolerates a missing onError; never
recovers after tripping (host must remount).
- ParagraphFooter mount contract: mount() returns {update, unmount},
throws without an element; execution-time text incl. anonymous user,
(outdated) suffix, and invalid-duration "outdated"; elapsed-time while
running; update() re-renders in place; unmount() empties the host.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Assisted-By: Claude <noreply@anthropic.com>
… to 9.5.7 First step of aligning the zeppelin-react toolchain with the node 22 runtime: typescript 4.9.5 predates the DefinitelyTyped support window, blocking @types/node 20+ and therefore vitest 4, whose 4.1.8 release is the only fix for the critical advisory GHSA-5xrq-8626-4rwp currently failing the npm-audit CI job. Aligns with the Angular workspace's typescript ~5.9.3; ts-loader 9.4.0 was a TS4-era pin. Verified: webpack production build, 13/13 vitest tests, eslint clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Assisted-By: Claude <noreply@anthropic.com>
Aligns the ambient node typings with the actual runtime (.nvmrc and CI run node 22.21.1, zeppelin-web-angular declares engines.node >=22.12.0). Requires the preceding typescript 5.9 bump; @types/node 20+ is outside DefinitelyTyped's support window for TS 4.9. Unblocks vitest 4, whose peer range rejects @types/node 18. Verified: webpack production build, 13/13 vitest tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Assisted-By: Claude <noreply@anthropic.com>
vitest <4.1.0 is flagged critical by GHSA-5xrq-8626-4rwp (arbitrary file read/execute while the Vitest UI server is listening), which fails the npm-audit CI gate (--audit-level=high). No 3.x backport exists; 4.1.8 is the fixed release, unblocked by the preceding typescript 5.9 and @types/node 22 bumps. We only run `vitest run` (no UI server), so the practical exposure was nil, but the gate is metadata-based. Also applies the non-breaking `npm audit fix` for ws (GHSA-58qx-3vcg-4xpx). Remaining: one moderate uuid advisory via sockjs/webpack-dev-server with no compatible upstream fix; it does not trip the high-level gate. Verified: 13/13 vitest tests, `npm audit --audit-level=high` exits 0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Assisted-By: Claude <noreply@anthropic.com>
…s on CI The previous lockfile was updated incrementally on macOS, which hits the known npm bug where dependencies of platform-skipped optional packages (rolldown/vite 8 wasm fallback chain: @emnapi/core, @emnapi/runtime, @noble/hashes) are not recorded. Linux runners then fail `npm ci` with EUSAGE "Missing: ... from lock file" before the audit even runs. Rebuild the lockfile from scratch and complete it with `npm install --package-lock-only`, which resolves the full ideal tree platform-independently. Verified with the exact CI sequence: `npm ci --ignore-scripts && npm audit --audit-level=high` exits 0; 13/13 vitest tests pass; lockfileVersion 3 and pinned versions (typescript 5.9.3, vitest 4.1.8) unchanged. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Assisted-By: Claude <noreply@anthropic.com>
…ci compat npm 10.9.x's arborist drops two kinds of entries from the lockfile it writes, and its own `npm ci` then rejects that lockfile with EUSAGE "Missing: ... from lock file" on CI: - deps of platform-skipped optional packages (the vite 8/rolldown wasm fallback chain: @emnapi/core, @emnapi/runtime) - nested duplicate versions behind peerOptional ranges (jsdom 29 → html-encoding-sniffer → @exodus/bytes peerOptional @noble/hashes@2.x vs pkijs' @noble/hashes@^1.4.0) Regenerate the lockfile with npm 11 (fixed arborist) inside a node:22 Linux container matching the CI runner, then validate it with the CI's own npm 10.9.8: `npm ci --ignore-scripts && npm audit --audit-level=high` exits 0 on both Linux and macOS. lockfileVersion 3 and all pinned versions (typescript 5.9.3, vitest 4.1.8) unchanged; 13/13 vitest tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Assisted-By: Claude <noreply@anthropic.com>
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
1 participant
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.
What is this PR for?
zeppelin-web-angular ships React islands via Webpack Module Federation (PublishedParagraph pilot). This PR promotes that ad-hoc integration into reusable mount infrastructure and migrates the notebook paragraph footer as its first consumer, behind a
?reactFooter=truequery-param gate.share/react-mount/:ReactMountDirective,ReactRemoteLoaderService, and aReactMountHandlecontract (mount(element, props)returning{ update, unmount }) so prop changes update the React root in place instead of remounting.remoteEntry.jsis loaded once per page (cached promise with failure eviction); each directive instance owns an isolated React root.ParagraphFootercomponent (zeppelin-react) matching the Angular footer's execution-time and elapsed-time behavior, wrapped in an error boundary.?reactFooter=true. Without the flag, rendering is unchanged. With the flag, remote load/mount/update errors — and React render/lifecycle errors caught by the error boundary — fall back to the Angular footer for that paragraph only.react-footer.spec.ts) and a vitest unit-test harness for zeppelin-react covering the error boundary and the mount contract.projects/zeppelin-react: typescript 4.9.5 → 5.9.3 (matching the Angular workspace), @types/node 18 → 22 (matching the node 22 runtime), vitest 4.1.8 — the latter fixes the critical advisory GHSA-5xrq-8626-4rwp flagged by the npm-audit CI job.The existing PublishedParagraph pilot remains on its current loader; this PR only types its
container.get()call. Migrating it to the new directive is a follow-up.What type of PR is it?
Improvement
Todos
react-mount/)ParagraphFooter+ error boundary?reactFooter=truegate with per-paragraph Angular fallbackWhat is the Jira issue?
How should this be tested?
Verified locally:
npx playwright test --project=chromium react-footer— 5/5 passed against a local Zeppelin server (0.13.0-SNAPSHOT), including the remote-load-failure fallback case.cd projects/zeppelin-react && npm test— 13/13 vitest unit tests (error boundary behavior,mount()contract incl. update/unmount, outdated/elapsed formatting with a pinned clock).npm audit --audit-level=highexits 0 inprojects/zeppelin-react(same gate as the npm-audit CI job). One moderate uuid advisory remains via sockjs/webpack-dev-server with no compatible upstream fix; it does not trip the high-level gate.eslint/prettierclean on changed files;tsc --noEmitclean (also covers what thetranspileOnlywebpack build skips).Manual:
?reactFooter=true→ footer renders from React (data-testid="react-paragraph-footer"),remoteEntry.jsrequested once.remoteEntry.jsin DevTools → the paragraph falls back to the Angular footer with a console diagnostic.Note on coverage: the React pieces are unit-tested via a new self-contained vitest setup in
projects/zeppelin-react(vitest 4.1.8). The Angular-side pieces (ReactMountDirective,ReactRemoteLoaderService) are covered through E2E only, since zeppelin-web-angular has no unit-test harness (zero.spec.tsundersrc/, no test target in angular.json). Happy to add Angular unit tests if a harness lands or committers prefer a different approach.Screenshots (if appropriate)
The React footer is intended to match the existing footer's rendering (same text and layout; styles ported to a scoped CSS class).
Questions:
date-fns(MIT) toprojects/zeppelin-react's package.json and lockfile (the Angular app already depends on it), plus dev-only test dependencies (vitest, jsdom, Testing Library). The lockfile diff includes npm peer-flag normalization fromnpm install.projects/zeppelin-react/README.md.