Fix hydrate_on scheduling review feedback

Author: justin808

packages/react-on-rails/src/ClientRenderer.ts

@@ -97,6 +97,18 @@ function teardownEntry(entry: RenderedEntry, domNodeId: string): void {
   }
 }
 
+function teardownErrorLabel(entry: RenderedEntry, domNodeId: string): string {
+  if (entry.kind === 'renderer') {
+    return `Error in renderer teardown for dom node "${domNodeId}":`;
+  }
+
+  if (entry.kind === 'scheduled') {
+    return `Error canceling scheduled render for dom node "${domNodeId}":`;
+  }
+
+  return `Error unmounting component for dom node "${domNodeId}":`;
+}
+
 function initializeStore(el: Element, railsContext: RailsContext): void {
   const name = el.getAttribute(REACT_ON_RAILS_STORE_ATTRIBUTE) || '';
   const props = el.textContent !== null ? (JSON.parse(el.textContent) as Record<string, unknown>) : {};
@@ -130,6 +142,9 @@ function hydrateOnForEl(el: Element): HydrateOnMode {
 
 function scheduleWhenVisible(domNode: Element, callback: () => void): () => void {
   if (typeof IntersectionObserver === 'undefined') {
+    console.warn(
+      '[react-on-rails] IntersectionObserver is not available. Falling back to immediate hydration for hydrate_on: :visible.',
+    );
     const timeoutId = window.setTimeout(callback, 0);
     return () => window.clearTimeout(timeoutId);
   }
@@ -166,7 +181,7 @@ function scheduleWhenIdle(callback: () => void): () => void {
     };
   }
 
-  const timeoutId = window.setTimeout(callback, 1);
+  const timeoutId = window.setTimeout(callback, 50);
   return () => window.clearTimeout(timeoutId);
 }
 
@@ -183,11 +198,19 @@ function scheduleHydration(hydrateOn: HydrateOnMode, domNode: Element, callback:
   return () => {};
 }
 
-function raiseRenderError(componentName: string, error: unknown): never {
+function prepareRenderError(componentName: string, error: unknown): Error {
   const renderError = error as Error;
   console.error(renderError.message);
   renderError.message = `ReactOnRails encountered an error while rendering component: ${componentName}. See above error message.`;
-  throw renderError;
+  return renderError;
+}
+
+function raiseRenderError(componentName: string, error: unknown): never {
+  throw prepareRenderError(componentName, error);
+}
+
+function reportRenderError(componentName: string, error: unknown): void {
+  console.error(prepareRenderError(componentName, error).message);
 }
 
 // Result of attempting renderer delegation. Core carries the raw RendererResult (which may still be
@@ -339,11 +362,7 @@ function renderElement(el: Element, railsContext: RailsContext): void {
           // Surface the failure unconditionally (matching unmountAllComponents) so a teardown/unmount
           // error on node replacement is as visible as one on page unload, using the same greppable
           // labels. We still continue: the old mount may leak, but the new node must be rendered.
-          const label =
-            existing.kind === 'renderer'
-              ? `Error in renderer teardown for dom node "${domNodeId}":`
-              : `Error unmounting component for dom node "${domNodeId}":`;
-          console.error(label, unmountError);
+          console.error(teardownErrorLabel(existing, domNodeId), unmountError);
         }
         renderedRoots.delete(domNodeId);
       }
@@ -411,7 +430,7 @@ You should return a React.Component always for the client side entry point.`);
           mountReactRoot();
         } catch (scheduledError) {
           renderedRoots.delete(domNodeId);
-          raiseRenderError(name, scheduledError);
+          reportRenderError(name, scheduledError);
         }
       };
 
@@ -486,15 +505,7 @@ function unmountAllComponents(): void {
     try {
       teardownEntry(entry, domNodeId);
     } catch (error) {
-      // Use the same label as the async-rejection path so renderer-teardown failures are greppable
-      // whether the teardown threw synchronously (here) or rejected (invokeRendererTeardown).
-      let label = `Error unmounting component for dom node "${domNodeId}":`;
-      if (entry.kind === 'renderer') {
-        label = `Error in renderer teardown for dom node "${domNodeId}":`;
-      } else if (entry.kind === 'scheduled') {
-        label = `Error canceling scheduled render for dom node "${domNodeId}":`;
-      }
-      console.error(label, error);
+      console.error(teardownErrorLabel(entry, domNodeId), error);
     }
   });
   renderedRoots.clear();

packages/react-on-rails/tests/ClientRenderer.test.ts

@@ -800,6 +800,117 @@ describe('ClientRenderer', () => {
 
       expect(mockHydrateOrRender).not.toHaveBeenCalled();
     });
+
+    it('reports scheduled render errors without throwing from the deferred callback', () => {
+      type ObserverCallback = ConstructorParameters<typeof IntersectionObserver>[0];
+      let observerCallback!: ObserverCallback;
+      const renderError = new Error('scheduled render boom');
+      const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
+      const mockHydrateOrRender = require('../src/reactHydrateOrRender.ts').default as jest.Mock;
+      mockHydrateOrRender.mockImplementationOnce(() => {
+        throw renderError;
+      });
+
+      class MockIntersectionObserver {
+        disconnect = jest.fn();
+
+        observe = jest.fn();
+
+        constructor(callback: ObserverCallback) {
+          observerCallback = callback;
+        }
+      }
+
+      Object.defineProperty(globalThis, 'IntersectionObserver', {
+        configurable: true,
+        value: MockIntersectionObserver,
+      });
+
+      setupScheduledComponentDom('hydrate-visible-error', 'visible');
+      renderComponent('hydrate-visible-error');
+
+      expect(() => {
+        observerCallback(
+          [{ isIntersecting: true, intersectionRatio: 1 }] as IntersectionObserverEntry[],
+          {} as IntersectionObserver,
+        );
+      }).not.toThrow();
+      expect(consoleErrorSpy).toHaveBeenCalledWith('scheduled render boom');
+      expect(consoleErrorSpy).toHaveBeenCalledWith(
+        'ReactOnRails encountered an error while rendering component: TestComponent. See above error message.',
+      );
+      consoleErrorSpy.mockRestore();
+    });
+
+    it('warns when visible hydration falls back without IntersectionObserver', () => {
+      jest.useFakeTimers();
+      const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
+      setupScheduledComponentDom('hydrate-visible-fallback', 'visible');
+      const mockHydrateOrRender = require('../src/reactHydrateOrRender.ts').default as jest.Mock;
+
+      renderComponent('hydrate-visible-fallback');
+
+      expect(consoleWarnSpy).toHaveBeenCalledWith(
+        '[react-on-rails] IntersectionObserver is not available. Falling back to immediate hydration for hydrate_on: :visible.',
+      );
+      expect(mockHydrateOrRender).not.toHaveBeenCalled();
+
+      jest.runOnlyPendingTimers();
+      expect(mockHydrateOrRender).toHaveBeenCalledTimes(1);
+
+      consoleWarnSpy.mockRestore();
+      jest.useRealTimers();
+    });
+
+    it('uses a short idle fallback delay when requestIdleCallback is unavailable', () => {
+      jest.useFakeTimers();
+      setupScheduledComponentDom('hydrate-idle-fallback', 'idle');
+      const mockHydrateOrRender = require('../src/reactHydrateOrRender.ts').default as jest.Mock;
+
+      renderComponent('hydrate-idle-fallback');
+
+      jest.advanceTimersByTime(49);
+      expect(mockHydrateOrRender).not.toHaveBeenCalled();
+
+      jest.advanceTimersByTime(1);
+      expect(mockHydrateOrRender).toHaveBeenCalledTimes(1);
+
+      jest.useRealTimers();
+    });
+
+    it('logs scheduled cancellation errors with the scheduled label during node replacement', () => {
+      const disconnectError = new Error('scheduled disconnect boom');
+      const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
+
+      class MockIntersectionObserver {
+        disconnect = jest.fn(() => {
+          throw disconnectError;
+        });
+
+        observe = jest.fn();
+      }
+
+      Object.defineProperty(globalThis, 'IntersectionObserver', {
+        configurable: true,
+        value: MockIntersectionObserver,
+      });
+
+      const targetNode = setupScheduledComponentDom('hydrate-visible-replace-label', 'visible');
+      renderComponent('hydrate-visible-replace-label');
+
+      targetNode.remove();
+      const replacementNode = document.createElement('div');
+      replacementNode.id = 'hydrate-visible-replace-label';
+      document.body.appendChild(replacementNode);
+
+      expect(() => renderComponent('hydrate-visible-replace-label')).not.toThrow();
+      expect(consoleErrorSpy).toHaveBeenCalledWith(
+        'Error canceling scheduled render for dom node "hydrate-visible-replace-label":',
+        disconnectError,
+      );
+
+      consoleErrorSpy.mockRestore();
+    });
   });
 
   describe('page unload cleanup (React-root cleanup)', () => {