This is an automated email from the ASF dual-hosted git repository.
tbonelee 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 d64897a0ab [ZEPPELIN-6565] Re-enter Angular zone for React remote
callbacks
d64897a0ab is described below
commit d64897a0abdb4c9f720cf26faf95861c0e117ef1
Author: Minho Jang <[email protected]>
AuthorDate: Sat Aug 15 17:58:32 2026 +0900
[ZEPPELIN-6565] Re-enter Angular zone for React remote callbacks
### What is this PR for?
This PR fixes `ReactMountDirective` so callbacks invoked by a mounted React
remote re-enter the Angular zone before calling back into the Angular host.
`ReactMountDirective` mounts and updates React remotes inside
`ngZone.runOutsideAngular()`. The directive already used `ngZone.run()` for
errors reported through its own `reportError()` path, but props were passed to
the remote unchanged. As a result, a remote such as the React paragraph footer
could call `props.onError()` directly from outside the Angular zone.
For an Angular `OnPush` host component, that can mark the component dirty
without scheduling change detection, delaying the fallback UI until some later
zone-scheduled work occurs.
This change wraps host callbacks as props enter `ReactMountDirective`,
while keeping the original props (`latestRawProps`) available for
directive-internal error reporting. The wrapped props (`latestProps`) are
passed to both `mount()` and `update()`, so React remote callbacks consistently
run through the Angular zone boundary.
### What type of PR is it?
Bug Fix
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-6565
### How should this be tested?
Run the shell unit tests:
```sh
cd zeppelin-web-angular
npm run test:shell
```
### Questions:
* Does the license files need to update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
Closes #5417 from miinhho/fix/react-lifecycle-outside.
Signed-off-by: ChanHo Lee <[email protected]>
---
.../react-mount/react-mount.directive.spec.ts | 58 +++++++++++++++++++++-
.../app/share/react-mount/react-mount.directive.ts | 25 +++++++++-
2 files changed, 80 insertions(+), 3 deletions(-)
diff --git
a/zeppelin-web-angular/src/app/share/react-mount/react-mount.directive.spec.ts
b/zeppelin-web-angular/src/app/share/react-mount/react-mount.directive.spec.ts
index 7a9862707f..71c5e64e30 100644
---
a/zeppelin-web-angular/src/app/share/react-mount/react-mount.directive.spec.ts
+++
b/zeppelin-web-angular/src/app/share/react-mount/react-mount.directive.spec.ts
@@ -14,7 +14,7 @@ import { ElementRef, NgZone, SimpleChange } from
'@angular/core';
import { describe, expect, it, vi } from 'vitest';
import { ReactRemoteLoaderService } from './react-remote-loader.service';
-import { ReactExposedModule, ReactMountHandle, ReactProps } from
'./react-mount-handle';
+import { ReactExposedModule, ReactHostCallbacks, ReactMountHandle, ReactProps
} from './react-mount-handle';
import { ReactMountDirective } from './react-mount.directive';
describe('ReactMountDirective', () => {
@@ -62,4 +62,60 @@ describe('ReactMountDirective', () => {
expect(unmount).toHaveBeenCalledOnce();
});
+
+ it('re-enters the Angular zone for callbacks invoked by the React remote',
async () => {
+ const host = new ElementRef<HTMLElement>(document.createElement('div'));
+ const ngZone = new NgZone({});
+ let mountedProps: (ReactProps & ReactHostCallbacks) | undefined;
+ let updatedProps: (ReactProps & ReactHostCallbacks) | undefined;
+ const update = vi.fn((props: ReactProps & ReactHostCallbacks) => {
+ updatedProps = props;
+ });
+ const mountHandle: ReactMountHandle = {
+ update,
+ unmount: vi.fn()
+ };
+ const remote: ReactExposedModule = {
+ mount: (_element: HTMLElement, props: ReactProps & ReactHostCallbacks)
=> {
+ mountedProps = props;
+ return mountHandle;
+ }
+ };
+ const loadModule = vi.fn(async <T>(): Promise<T> => remote as T);
+ const loader = { loadModule } as Pick<ReactRemoteLoaderService,
'loadModule'>;
+ const zoneStates: boolean[] = [];
+ const onMountError = vi.fn(() => {
+ zoneStates.push(NgZone.isInAngularZone());
+ });
+ const onUpdateError = vi.fn(() => {
+ zoneStates.push(NgZone.isInAngularZone());
+ });
+ const directive = new ReactMountDirective(host, ngZone, loader as
ReactRemoteLoaderService);
+
+ directive.module = 'paragraph-footer';
+ directive.reactProps = { onError: onMountError };
+ directive.ngOnChanges({
+ module: new SimpleChange(undefined, directive.module, true),
+ reactProps: new SimpleChange(undefined, directive.reactProps, true)
+ });
+ await vi.waitFor(() => expect(mountedProps).toBeDefined());
+
+ ngZone.runOutsideAngular(() => {
+ mountedProps!.onError!(new Error('mount remote failed'));
+ });
+
+ directive.reactProps = { onError: onUpdateError };
+ directive.ngOnChanges({
+ reactProps: new SimpleChange({ onError: onMountError },
directive.reactProps, false)
+ });
+
+ ngZone.runOutsideAngular(() => {
+ updatedProps!.onError!(new Error('update remote failed'));
+ });
+
+ expect(onMountError).toHaveBeenCalledOnce();
+ expect(onUpdateError).toHaveBeenCalledOnce();
+ expect(update).toHaveBeenCalledOnce();
+ expect(zoneStates).toEqual([true, true]);
+ });
});
diff --git
a/zeppelin-web-angular/src/app/share/react-mount/react-mount.directive.ts
b/zeppelin-web-angular/src/app/share/react-mount/react-mount.directive.ts
index c93c168001..a28a575b7e 100644
--- a/zeppelin-web-angular/src/app/share/react-mount/react-mount.directive.ts
+++ b/zeppelin-web-angular/src/app/share/react-mount/react-mount.directive.ts
@@ -40,6 +40,7 @@ export class ReactMountDirective implements OnChanges,
OnDestroy {
@Input('zeppelin-react-mount') module!: string;
@Input() reactProps: ReactProps & ReactHostCallbacks = {};
+ private latestRawProps: ReactProps & ReactHostCallbacks = {};
private latestProps: ReactProps & ReactHostCallbacks = {};
private destroyed = false;
private loading = false;
@@ -53,7 +54,8 @@ export class ReactMountDirective implements OnChanges,
OnDestroy {
) {}
ngOnChanges(changes: SimpleChanges): void {
- this.latestProps = this.reactProps ?? {};
+ this.latestRawProps = this.reactProps ?? {};
+ this.latestProps = this.withHostCallbacks(this.latestRawProps);
if (changes.module && !changes.module.firstChange && this.mountedModule) {
// Module swap after first mount is unsupported. Report via onError
@@ -128,7 +130,7 @@ export class ReactMountDirective implements OnChanges,
OnDestroy {
}
private reportError(error: unknown): void {
- const onError = this.latestProps.onError;
+ const onError = this.latestRawProps.onError;
if (typeof onError === 'function') {
// Re-enter the Angular zone so onError handlers can safely mutate
// host state and trigger change detection. React lifecycle callbacks
@@ -146,4 +148,23 @@ export class ReactMountDirective implements OnChanges,
OnDestroy {
console.error('[ReactMountDirective]', error);
}
}
+
+ private withHostCallbacks(props: ReactProps & ReactHostCallbacks):
ReactProps & ReactHostCallbacks {
+ const onError = props.onError;
+ if (typeof onError !== 'function') {
+ return props;
+ }
+ return {
+ ...props,
+ onError: (error: unknown): void => {
+ this.ngZone.run(() => {
+ try {
+ onError(error);
+ } catch {
+ /* swallow callback errors; they shouldn't loop */
+ }
+ });
+ }
+ };
+ }
}
