[ZEPPELIN-6630] Render the /configuration table through a React remote behind a flag

[kimyenac]

What is this PR for?

Renders the /configuration table through the React remote behind a flag, and adds the e2e coverage the page was missing. ZEPPELIN-6630 recommends doing ZEPPELIN-6363 first or alongside, so both are here: the safety net first, then the change it protects. [ZEPPELIN-6363] stands on its own, its spec passes with none of the port applied, so the two split cleanly if you would rather take them separately. The last two commits are @voidmatcha's, cherry-picked from the branch offered in review: e2e for the Angular fallback, and the docs that list the React surfaces.

ZEPPELIN-6363, first commit. The page had no spec of its own; what existed touched it only in passing through header navigation and the user menu. The new spec covers the header title and the security note, the Name and Value table, and that every entry carries a name while a value may legitimately be empty. Two assertions go past structure and pin behaviour the component owns: the entries come out sorted by name, which is the sort in getAllConfig(), and they survive a reload. The page model selects a shared configuration-table id that this commit puts on the Angular table and the second commit's React branch renders too, so the spec keeps working once the React table takes over. Nothing about this commit depends on the port: its spec passes with none of the port applied.

ZEPPELIN-6630, second commit. The suggested boundary is kept: ConfigurationService still fetches and sorts, zeppelin-page-header stays where it is, routing is untouched, and only the table inside .content moves. The remote receives the sorted [string, string][] and owns no state, so this does not wait on the host/remote state-sharing decision.

• The flag goes through ReactFeatureService from ZEPPELIN-6564, as a new configurationTable surface reading ?reactConfiguration. No new parsing.
• Both branches render the shared configuration-table id, and the mount host around it is what tells them apart. An onError from the remote falls back to the Angular table for the rest of the session.
• The props getter is memoized on configEntries, the only input that changes. shallowEquals in paragraph.component.ts is a private helper of that component; rather than copy it, a single reference compare does the job here.
• queryParamMap is subscribed rather than read once, because navigating between /configuration and /configuration?reactConfiguration reuses the component. Replacing the subscription with a snapshot read fails the parity spec.

On visual consistency: the remote renders antd's Table inside ZeppelinThemeProvider, so it follows the shell's theme rather than the shell's stylesheets. Measured against the Angular table, font size, cell padding and background match in both themes. Column headers were the one difference visible side by side, since antd draws them at 600 and ng-zorro at 500, so the surface passes fontWeightStrong through the provider's token prop. Row height still differs by 1px in light mode, which is a library default and left alone.

Two things worth raising for the surfaces that follow:

• Every host component now repeats the props memoization. Doing it inside ReactMountDirective, which could skip handle.update() when the incoming props are shallow-equal, would remove that from each call site. It felt out of scope here, so it is only a suggestion.
• src/pages/index.ts is not updated. Nothing imports that barrel, and export * from a second page conflicts on mount, which is inherent to the mount-per-module contract.

projects/zeppelin-react/src/test-setup.ts gains a matchMedia stub: jsdom implements none and antd's responsive observer calls it while rendering, so Table throws and the error boundary renders nothing. Any later spec touching an antd component would hit the same wall.

What type of PR is it?

Improvement

Todos

None

What is the Jira issue?

• https://issues.apache.org/jira/browse/ZEPPELIN-6630
• https://issues.apache.org/jira/browse/ZEPPELIN-6363

How should this be tested?

• Playwright, chromium: the two new specs under e2e/tests/workspace/configuration/, 13 tests, no retries and no flakes. react-footer.spec.ts, published-paragraph.spec.ts and dark-mode.spec.ts re-run for regressions, 23 tests green.
• vitest: projects/zeppelin-react at 43 including the new ConfigurationTable.spec.tsx, and npm run test:shell at 4.
• Production builds: the remote (ConfigurationTable shows up in remoteEntry.js) and ng build --configuration production.
• Each new assertion was checked by breaking what it covers. Reversing the row order in the remote fails the parity spec; removing the sort in getAllConfig() fails the ZEPPELIN-6363 sort spec; replacing the query param subscription with a snapshot read fails the parity spec; dropping !this.reactTableFailed from shouldUseReactTable fails the fallback spec and nothing else; removing the matchMedia stub fails four vitest specs.
• The flip the ZEPPELIN-6363 spec exists to survive: with configurationTable.defaultEnabled set to true, so the page serves the React table by default, all seven of its tests still pass.
• The fallback spec's waitForRequest guard: renaming the surface's queryParam so ?reactConfiguration=true never resolves makes it time out. Without the guard the assertions would have passed on the default Angular branch.
• Manually in both themes at /#/configuration and /#/configuration?reactConfiguration=true.

Firefox and WebKit were not run locally.

Screenshots (if appropriate)

The two tables side by side are hard to tell apart, which is the intent. In light mode the header weight now matches and only row height differs, by 1px.

Questions:

• Does the license files need to update? No
• Is there breaking changes for older versions? No, the flag defaults to off and the Angular table is unchanged when it is
• Does this needs documentation? No

[tbonelee]

Looks good overall. One thing: the safety-net spec's selector is pinned to the Angular implementation, so it will not survive the flip it exists to protect. Happy to merge once the comment below is addressed.

[tbonelee]

Pinned to nz-table, this locator takes all six tests in this spec down the moment the flag defaults on or the Angular branch is removed. e2e/AGENTS.md also points at a shared data-testid that both implementations render at a seam.

If the first commit adds a neutral id to the Angular table and the second commit's React branch renders the same id, the commits stay independent and the spec survives the flip.

[kimyenac]

Good catch, fixed in bced864.

The first commit now puts data-testid="configuration-table" on the Angular
table and the page model selects that, and the second commit's React branch
renders the same id. The flag spec no longer needs a branch-specific table id
either: both branches render configuration-table, and the mount host around it
is what tells them apart, so [data-testid="react-configuration-table"] [data-testid="configuration-table"]
scopes an assertion to the remote when it needs to be.

Checked the flip it is meant to survive: with configurationTable.defaultEnabled
flipped to true, so the page serves the React table by default, all seven tests
in the structure spec still pass. The 6363 commit also still stands alone, its
spec passes with none of the port applied.

[voidmatcha]

The onError path that flips reactTableFailed back to the Angular table doesn't seem to be covered yet. It's in ZEPPELIN-6630's scope and both existing React surfaces test it (react-footer, published-paragraph), so I suspect it just got missed rather than skipped.

I took a stab at one in case it's useful, on a branch along with the doc spots that list the React surfaces and haven't caught up to the third:
voidmatcha/zeppelin@72a185d...622080f

Take it, rewrite it, or tell me it belongs somewhere else.

It follows the published-paragraph test including its waitForRequest guard, since Angular is the default branch here and without it the assertions pass even when the flag never took effect. I ran the configuration suite against a local Zeppelin to check: 13/13, and dropping !this.reactTableFailed from shouldUseReactTable makes it the only failure.

[kimyenac]

Taking both commits as they are, cherry-picked so the authorship stays with you.

It was a miss, not a decision. I wrote the onError path and then never pointed
anything at it, while the two surfaces that came before both cover theirs. Same
for the docs: I grepped for every spot that lists the surfaces and the two your
commit touches are the only ones, so nothing is left behind.

I re-ran it here rather than take the numbers on faith. The suite is 13/13, and
dropping !this.reactTableFailed from shouldUseReactTable leaves your test as
the only failure, which matches what you saw.

I also wanted to see the waitForRequest guard earn its place, so I broke the
flag instead of the fallback, renaming the configurationTable surface's
queryParam so ?reactConfiguration=true never resolves. The guard times out on
the request and fails the test. Without it the assertions would have passed on
the default Angular branch, so it is load-bearing rather than ceremony. Good
catch, and thanks for writing it out instead of just flagging it.

One thing worth a second opinion: the paragraph/ line your README change adds to
the project structure fixes an omission that predates this PR, from ZEPPELIN-6428.
I kept it since it sits in the block being corrected and the block is wrong
without it, but say the word if you would rather it went to its own issue.

[voidmatcha]

I'd keep the paragraph/ line. Adding ConfigurationTable.tsx is what turns that block into a list of mount entries, and two of three reads worse than the one it listed before.

I checked whether @tbonelee's point actually holds. Flipping configurationTable.defaultEnabled to true so the React table renders by default, the structure spec still passes, all 7 of them.

One thing left in the PR body:

The page model deliberately selects zeppelin-configuration nz-table rather than the data-testid the second commit introduces

That describes the design from before the review, so it reads as though nothing was addressed. "12 tests" was 11 at the time too, and the new test made it accidentally correct.

[kimyenac]

Updated the PR body: it still described the pre-review design and the test count from before your commit. Thanks for catching both.

Keeping the paragraph/ line then, and the flip check matches what I got here as well.