Skip to content

[ZEPPELIN-6428] Add reusable React mount infrastructure and migrate paragraph footer behind a flag#5266

Open
tbonelee wants to merge 19 commits into
apache:masterfrom
tbonelee:ZEPPELIN-6428-react-mount-infrastructure
Open

[ZEPPELIN-6428] Add reusable React mount infrastructure and migrate paragraph footer behind a flag#5266
tbonelee wants to merge 19 commits into
apache:masterfrom
tbonelee:ZEPPELIN-6428-react-mount-infrastructure

Conversation

@tbonelee
Copy link
Copy Markdown
Contributor

@tbonelee tbonelee commented Jun 3, 2026

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=true query-param gate.

  • share/react-mount/: ReactMountDirective, ReactRemoteLoaderService, and a ReactMountHandle contract (mount(element, props) returning { update, unmount }) so prop changes update the React root in place instead of remounting. remoteEntry.js is loaded once per page (cached promise with failure eviction); each directive instance owns an isolated React root.
  • React ParagraphFooter component (zeppelin-react) matching the Angular footer's execution-time and elapsed-time behavior, wrapped in an error boundary.
  • Gate: ?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.
  • Playwright E2E (react-footer.spec.ts) and a vitest unit-test harness for zeppelin-react covering the error boundary and the mount contract.

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

  • Mount infrastructure (react-mount/)
  • React ParagraphFooter + error boundary
  • ?reactFooter=true gate with per-paragraph Angular fallback
  • Playwright E2E (5 cases incl. load-failure fallback)
  • vitest unit tests (error boundary spec, mount contract)

What 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).
  • eslint/prettier clean on changed files; tsc --noEmit clean.

Manual:

  • Open a notebook with ?reactFooter=true → footer renders from React (data-testid="react-paragraph-footer"), remoteEntry.js requested once.
  • Without the flag → the Angular footer renders as before.
  • Block remoteEntry.js in 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 3.x — vitest 4 conflicts with the pinned @types/node@18). The Angular-side pieces (ReactMountDirective, ReactRemoteLoaderService) are covered through E2E only, since zeppelin-web-angular has no unit-test harness (zero .spec.ts under src/, 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:

  • Does the license files need to update? Adds date-fns (MIT) to projects/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 from npm install.
  • Is there breaking changes for older versions? No. The footer change is opt-in via query param; default rendering is unchanged.
  • Does this needs documentation? No user-facing docs; developer docs updated in projects/zeppelin-react/README.md.

ChanHo Lee and others added 19 commits June 3, 2026 21:27
� 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant