This is an automated email from the ASF dual-hosted git repository.
voidmatcha pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new 5c414a717d [ZEPPELIN-6726] Add a React notebook adapter for the
host-provided NotebookPort
5c414a717d is described below
commit 5c414a717dfb49892737ad9695bbf9373cc2e08c
Author: κΉμλ <[email protected]>
AuthorDate: Thu Oct 8 18:50:54 2026 +0900
[ZEPPELIN-6726] Add a React notebook adapter for the host-provided
NotebookPort
### What is this PR for?
Add the React notebook adapter that binds the host-provided `NotebookPort`
(the `core` mount prop) to the React remote. It is the read-only part of the
adapter. Commands follow once the contract defines them (see below).
`projects/zeppelin-react/src/notebook/NotebookCoreProvider.tsx`:
* `NotebookCoreProvider` puts the exact port object it receives into
context. It does not create a Core, wrap the port or copy its state, so the
host stays the only owner of notebook state.
* `useNotebookCore()` returns that port. Outside the provider it throws
instead of falling back to a local Core.
* `useNotebookSnapshot()` is `useSyncExternalStore(core.subscribe,
core.getSnapshot)`. React re-renders only when the port returns a different
snapshot reference.
* `useNotebookSelector(selector)` re-renders only when the selected value
changes (`Object.is`).
**Import boundary.** No new rule was needed. The existing notebook-core
boundary check (`test/notebook-core/import-boundary.ts`, run by
`test:notebook-core`) already discovers React modules that depend on the
contract, and walks their dependencies for:
* `<at>zeppelin/sdk`, `<at>zeppelin/services/`, `rxjs`, `axios`, WebSocket
and HTTP modules;
* the `fetch` / `WebSocket` / `XMLHttpRequest` globals;
* runtime (non-type) core imports.
So the adapter is checked as soon as it imports the contract type. The
existing SDK-backed result renderers do not depend on the contract, so they are
not swept in. This PR adds one parameterized case: an SDK, rxjs WebSocket,
runtime core or `fetch` dependency added to the adapter fails the check.
**Not in this PR:**
* **Semantic commands ("one user intent dispatches one semantic
command"):** `NotebookPort` on `master` exposes only `getSnapshot` and
`subscribe`. Defining commands belongs to the contract work under ZEPPELIN-6669
/ ZEPPELIN-6687, so this PR does not invent them. The command dispatch and its
count checks follow once the contract defines them. Happy to adjust the split
in review.
* **Exposing or mounting the adapter:** nothing imports it yet. Adding it
to `main.ts` or the webpack `exposes`, and mounting it on the notebook URL, is
ZEPPELIN-6727.
### What type of PR is it?
Improvement
### Todos
* [x] Bind the exact host-provided port and read snapshots through
`useSyncExternalStore`
* [x] Re-render only on a new snapshot reference or selected value
* [x] Hold one subscription across mount, unmount, StrictMode and a port
change
* [x] Pin that a prohibited import in the adapter fails the boundary check
* [ ] Semantic commands, once the `NotebookPort` contract defines them
### What is the Jira issue?
[ZEPPELIN-6726](https://issues.apache.org/jira/browse/ZEPPELIN-6726)
### How should this be tested?
```bash
cd zeppelin-web-angular
npm run test:react
npm run typecheck:react
npm run test:notebook-core
npm run lint:react
npm run build:react
```
All passed locally:
* `test:react`: 16 files / 114 tests, including the 9 new adapter specs;
* `test:notebook-core`: 118 tests, including the 4 new boundary cases;
* `typecheck:react`, `typecheck:notebook-core`, `lint:react`,
`build:react`, and prettier.
The adapter specs cover:
* port identity;
* the throw outside the provider;
* rendering a published snapshot;
* no re-render on the same snapshot reference;
* unsubscribe on unmount and resubscribe on remount;
* one subscription under StrictMode double mounting;
* no resubscribe on a same-port re-render;
* moving to a new port;
* the selector re-rendering only on a changed value.
**Each spec catches the defect it targets.** I broke the adapter on purpose
and checked that the failure was an assertion, not a missing import:
| Broken adapter | Failing specs |
| --- | --- |
| selector derived from the whole snapshot | the selector spec |
| local port returned outside the provider | the throw spec |
| subscription without cleanup | the unmount, StrictMode and port-change
specs |
| snapshot copied into React state | the same-reference and port-change
specs |
**The real boundary check fails on a prohibited import.** Adding `import
'<at>zeppelin/sdk';` to the adapter makes `checks all production React
consumers of the notebook contract` fail with `NotebookCoreProvider.tsx: import
<at>zeppelin/sdk`. Reverted.
### Screenshots (if appropriate)
N/A.
### Questions:
* Does the license files need to update? No. New files carry the ASF header.
* Is there breaking changes for older versions? No. Nothing existing
changes.
* Does this needs documentation? No. Nothing is exposed yet.
π€ Generated with [Claude Code](https://claude.com/claude-code)
Closes #5556 from kimyenac/ZEPPELIN-6726.
Signed-off-by: YONGJAE LEE <[email protected]>
---
.../src/notebook/NotebookCoreProvider.spec.tsx | 199 +++++++++++++++++++++
.../src/notebook/NotebookCoreProvider.tsx | 58 ++++++
.../test/notebook-core/import-boundary.spec.ts | 18 ++
3 files changed, 275 insertions(+)
diff --git
a/zeppelin-web-angular/projects/zeppelin-react/src/notebook/NotebookCoreProvider.spec.tsx
b/zeppelin-web-angular/projects/zeppelin-react/src/notebook/NotebookCoreProvider.spec.tsx
new file mode 100644
index 0000000000..d8f83b7637
--- /dev/null
+++
b/zeppelin-web-angular/projects/zeppelin-react/src/notebook/NotebookCoreProvider.spec.tsx
@@ -0,0 +1,199 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { act, StrictMode } from 'react';
+import { render, renderHook, screen } from '@testing-library/react';
+import type { NotebookCorePort, NotebookCoreSnapshot } from
'@zeppelin/notebook-core';
+import { describe, expect, it, vi } from 'vitest';
+import {
+ NotebookCoreProvider,
+ useNotebookCore,
+ useNotebookSelector,
+ useNotebookSnapshot
+} from './NotebookCoreProvider';
+
+// A host-owned port as the shell would pass it: a frozen object whose
snapshot reference
+// changes only when state changes.
+const createHostPort = (initial: NotebookCoreSnapshot) => {
+ let snapshot = initial;
+ const listeners = new Set<() => void>();
+ let subscribeCalls = 0;
+ const core: NotebookCorePort = Object.freeze({
+ getSnapshot: () => snapshot,
+ subscribe: (listener: () => void) => {
+ subscribeCalls += 1;
+ listeners.add(listener);
+ return () => {
+ listeners.delete(listener);
+ };
+ }
+ });
+ return {
+ core,
+ publish: (next: NotebookCoreSnapshot) => {
+ snapshot = next;
+ act(() => listeners.forEach(listener => listener()));
+ },
+ notifyWithoutChange: () => {
+ act(() => listeners.forEach(listener => listener()));
+ },
+ listenerCount: () => listeners.size,
+ subscribeCalls: () => subscribeCalls
+ };
+};
+
+const noteSnapshot: NotebookCoreSnapshot = { noteId: 'note-a', revisionId:
null };
+
+let renders = 0;
+const SnapshotProbe = () => {
+ renders += 1;
+ const snapshot = useNotebookSnapshot();
+ return (
+ <span data-testid="snapshot">
+ {snapshot.noteId}:{snapshot.revisionId ?? 'live'}
+ </span>
+ );
+};
+
+const renderWithPort = (core: NotebookCorePort) =>
+ render(
+ <NotebookCoreProvider core={core}>
+ <SnapshotProbe />
+ </NotebookCoreProvider>
+ );
+
+describe('NotebookCoreProvider', () => {
+ it('hands the tree the exact port object the host provided', () => {
+ const port = createHostPort(noteSnapshot);
+ const { result } = renderHook(() => useNotebookCore(), {
+ wrapper: ({ children }) => <NotebookCoreProvider
core={port.core}>{children}</NotebookCoreProvider>
+ });
+
+ expect(result.current).toBe(port.core);
+ });
+
+ it('throws outside the provider instead of falling back to a local Core', ()
=> {
+ // React reports the render error to console.error and, in development, as
a window error
+ // event that jsdom would print as uncaught. Both are expected here.
+ const consoleError = vi.spyOn(console, 'error').mockImplementation(() =>
undefined);
+ const preventReport = (event: ErrorEvent) => event.preventDefault();
+ window.addEventListener('error', preventReport);
+ try {
+ expect(() => renderHook(() => useNotebookCore())).toThrow(
+ 'useNotebookCore must be used inside NotebookCoreProvider'
+ );
+ } finally {
+ window.removeEventListener('error', preventReport);
+ consoleError.mockRestore();
+ }
+ });
+
+ it('renders each snapshot the port publishes', () => {
+ const port = createHostPort(noteSnapshot);
+ renderWithPort(port.core);
+ expect(screen.getByTestId('snapshot').textContent).toBe('note-a:live');
+
+ port.publish({ noteId: 'note-a', revisionId: 'revision-1' });
+
+
expect(screen.getByTestId('snapshot').textContent).toBe('note-a:revision-1');
+ });
+
+ it('does not re-render when the port notifies with the same snapshot
reference', () => {
+ const port = createHostPort(noteSnapshot);
+ renderWithPort(port.core);
+ const rendersBefore = renders;
+
+ port.notifyWithoutChange();
+
+ expect(renders).toBe(rendersBefore);
+ });
+
+ it('removes its subscription on unmount and holds one again after
remounting', () => {
+ const port = createHostPort(noteSnapshot);
+ const first = renderWithPort(port.core);
+ expect(port.listenerCount()).toBe(1);
+
+ first.unmount();
+ expect(port.listenerCount()).toBe(0);
+
+ renderWithPort(port.core);
+ expect(port.listenerCount()).toBe(1);
+ });
+
+ it('holds one subscription under StrictMode double mounting', () => {
+ const port = createHostPort(noteSnapshot);
+ render(
+ <StrictMode>
+ <NotebookCoreProvider core={port.core}>
+ <SnapshotProbe />
+ </NotebookCoreProvider>
+ </StrictMode>
+ );
+
+ expect(port.listenerCount()).toBe(1);
+ });
+
+ it('keeps its subscription when re-rendered with the same port', () => {
+ const port = createHostPort(noteSnapshot);
+ const { rerender } = renderWithPort(port.core);
+ const subscribeCallsBefore = port.subscribeCalls();
+
+ rerender(
+ <NotebookCoreProvider core={port.core}>
+ <SnapshotProbe />
+ </NotebookCoreProvider>
+ );
+
+ expect(port.subscribeCalls()).toBe(subscribeCallsBefore);
+ expect(port.listenerCount()).toBe(1);
+ });
+
+ it('moves its subscription to a new port the host passes', () => {
+ const previous = createHostPort(noteSnapshot);
+ const next = createHostPort({ noteId: 'note-b', revisionId: null });
+ const { rerender } = renderWithPort(previous.core);
+
+ rerender(
+ <NotebookCoreProvider core={next.core}>
+ <SnapshotProbe />
+ </NotebookCoreProvider>
+ );
+
+ expect(previous.listenerCount()).toBe(0);
+ expect(next.listenerCount()).toBe(1);
+ expect(screen.getByTestId('snapshot').textContent).toBe('note-b:live');
+ });
+});
+
+describe('useNotebookSelector', () => {
+ it('re-renders only when the selected value changes', () => {
+ const port = createHostPort(noteSnapshot);
+ let selectorRenders = 0;
+ const RevisionProbe = () => {
+ selectorRenders += 1;
+ return <span data-testid="revision">{useNotebookSelector(snapshot =>
snapshot.revisionId) ?? 'live'}</span>;
+ };
+ render(
+ <NotebookCoreProvider core={port.core}>
+ <RevisionProbe />
+ </NotebookCoreProvider>
+ );
+ const rendersBefore = selectorRenders;
+
+ port.publish({ noteId: 'note-b', revisionId: null });
+ expect(selectorRenders).toBe(rendersBefore);
+
+ port.publish({ noteId: 'note-b', revisionId: 'revision-1' });
+ expect(selectorRenders).toBe(rendersBefore + 1);
+ expect(screen.getByTestId('revision').textContent).toBe('revision-1');
+ });
+});
diff --git
a/zeppelin-web-angular/projects/zeppelin-react/src/notebook/NotebookCoreProvider.tsx
b/zeppelin-web-angular/projects/zeppelin-react/src/notebook/NotebookCoreProvider.tsx
new file mode 100644
index 0000000000..1ffb1ebec5
--- /dev/null
+++
b/zeppelin-web-angular/projects/zeppelin-react/src/notebook/NotebookCoreProvider.tsx
@@ -0,0 +1,58 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { createContext, ReactNode, useContext, useSyncExternalStore } from
'react';
+import type { NotebookCorePort, NotebookCoreRemoteProps, NotebookCoreSnapshot
} from '@zeppelin/notebook-core';
+
+const NotebookCoreContext = createContext<NotebookCorePort | null>(null);
+
+export type NotebookCoreProviderProps = NotebookCoreRemoteProps &
+ Readonly<{
+ children: ReactNode;
+ }>;
+
+/**
+ * Binds the host-provided port to the React tree. The adapter keeps the exact
object it
+ * receives: it never creates a Core, wraps the port or copies its state, so
the host stays
+ * the only owner of notebook state.
+ */
+export const NotebookCoreProvider = ({ core, children }:
NotebookCoreProviderProps) => (
+ <NotebookCoreContext.Provider
value={core}>{children}</NotebookCoreContext.Provider>
+);
+
+/** The host-provided port. Throws outside the provider rather than falling
back to a local Core. */
+export const useNotebookCore = (): NotebookCorePort => {
+ const core = useContext(NotebookCoreContext);
+ if (core === null) {
+ throw new Error('useNotebookCore must be used inside
NotebookCoreProvider');
+ }
+ return core;
+};
+
+/**
+ * The current snapshot. React re-renders only when the port returns a
different snapshot
+ * reference; the port keeps the same reference while state is unchanged.
+ */
+export const useNotebookSnapshot = (): NotebookCoreSnapshot => {
+ const core = useNotebookCore();
+ return useSyncExternalStore(core.subscribe, core.getSnapshot);
+};
+
+/**
+ * A view value derived from the snapshot. React re-renders only when the
selected value
+ * changes (`Object.is`), so a selector must return a primitive or a value the
snapshot
+ * already holds, never a new object or array per call.
+ */
+export const useNotebookSelector = <T,>(selector: (snapshot:
NotebookCoreSnapshot) => T): T => {
+ const core = useNotebookCore();
+ return useSyncExternalStore(core.subscribe, () =>
selector(core.getSnapshot()));
+};
diff --git a/zeppelin-web-angular/test/notebook-core/import-boundary.spec.ts
b/zeppelin-web-angular/test/notebook-core/import-boundary.spec.ts
index 40b4227221..befd64f9ae 100644
--- a/zeppelin-web-angular/test/notebook-core/import-boundary.spec.ts
+++ b/zeppelin-web-angular/test/notebook-core/import-boundary.spec.ts
@@ -479,6 +479,24 @@ describe('notebook core import boundary', () => {
);
}, 30_000);
+ it.each([
+ ["import '@zeppelin/sdk';", 'import @zeppelin/sdk'],
+ ["import { webSocket } from 'rxjs/webSocket';", 'import rxjs/webSocket'],
+ ["import * as core from '@zeppelin/notebook-core';", 'runtime notebook
core import'],
+ ["export const load = () => fetch('/api/notebook');", 'global fetch']
+ ])(
+ 'rejects %s in the production notebook adapter',
+ (addition, violation) => {
+ const root = resolve(zeppelinWebAngularRoot,
'projects/zeppelin-react/src');
+ const adapter = resolve(root, 'notebook/NotebookCoreProvider.tsx');
+ const source = `${readFileSync(adapter, 'utf8')}\n${addition}\n`;
+ const options = readCompilerOptions(resolve(root, '../tsconfig.json'));
+ const host = createFixtureHost(options, new Map([[adapter, source]]));
+ expect(findReactNotebookConsumerViolations([adapter], options,
host)).toContain(`${adapter}: ${violation}`);
+ },
+ 30_000
+ );
+
it('keeps the React contract dependent only on the public core entry point',
() => {
const path = reactNotebookCoreBoundaryFiles[1];
expect(findNotebookContractViolations(path, readFileSync(path,
'utf8'))).toEqual([]);
