feat: new dev profiler to get feedback on short updates

BREAKING CHANGE: The `triggerTREInvalidationPropNames` has been discontinued. The idea for this prop originated in the premise that many beginners would disregard the issue of passing literal props triggering many re-renders. But I realized it mostly frustrated expectations of newcomers that this library honors React components contract. So I have finally decided to discard the prop, and instead add a lightweight profiler (in dev mode only, of course), which warns the consumer of re-renders happening in a short period of time. You are now responsible for the invalidation of the TRenderEngine, so make sure you memoize the props if the controlling components is expected to re-render often.
meliorence · Jun 19, 2021 · 98fd749 · 98fd749
1 parent b2431d2
commit 98fd749
Show file tree

Hide file tree

Showing 21 changed files with 273 additions and 222 deletions.
diff --git a/apps/demo/src/components/UIHtmlDisplayMolecule.tsx b/apps/demo/src/components/UIHtmlDisplayMolecule.tsx
@@ -5,6 +5,15 @@ import UIDisplayLoadingAtom from './UIDisplayLoadingAtom';
 import useOnLinkPress from '../hooks/useOnLinkPress';
 import { useColorRoles } from '../theme/colorSystem';
 import { SYSTEM_FONTS } from '../constants';
+import { useMemo } from 'react';
+
+function renderRemoteLoadingView() {
+  return <UIDisplayLoadingAtom />;
+}
+
+const defaultTextProps = {
+  selectable: true
+};
 
 const UIHtmlDisplayMolecule = React.memo(
   ({
@@ -21,51 +30,57 @@ const UIHtmlDisplayMolecule = React.memo(
       onSelectUri
     ]);
     const { surface, softDivider } = useColorRoles();
-    const baseStyle = {
-      color: surface.content,
-      backgroundColor: surface.background,
-      //@ts-ignore
-      ...renderHtmlProps.baseStyle
-    };
+    const baseStyle = useMemo(
+      () => ({
+        color: surface.content,
+        backgroundColor: surface.background,
+        ...renderHtmlProps.baseStyle
+      }),
+      [renderHtmlProps.baseStyle, surface.background, surface.content]
+    );
     const sharedProps = {
       contentWidth,
       ...(renderHtmlProps as any),
-      renderersProps: {
-        ...renderHtmlProps.renderersProps,
-        a: {
-          onPress: onLinkPress,
-          ...renderHtmlProps.renderersProps?.a
-        },
-        img: {
-          enableExperimentalPercentWidth: true,
-          ...renderHtmlProps.renderersProps?.img
-        }
-      },
-      defaultTextProps: {
-        selectable: true
-      }
-    };
-    const mergedTagsStyles = {
-      ...sharedProps.tagsStyles,
-      hr: {
-        marginTop: 16,
-        marginBottom: 16,
-        ...sharedProps.tagsStyles?.hr,
-        height: 1,
-        backgroundColor: softDivider
-      },
-      html: {}
+      renderersProps: useMemo(
+        () => ({
+          ...renderHtmlProps.renderersProps,
+          a: {
+            onPress: onLinkPress,
+            ...renderHtmlProps.renderersProps?.a
+          },
+          img: {
+            enableExperimentalPercentWidth: true,
+            ...renderHtmlProps.renderersProps?.img
+          }
+        }),
+        [onLinkPress, renderHtmlProps.renderersProps]
+      ),
+      defaultTextProps
     };
+    const mergedTagsStyles = useMemo(
+      () => ({
+        ...sharedProps.tagsStyles,
+        hr: {
+          marginTop: 16,
+          marginBottom: 16,
+          ...sharedProps.tagsStyles?.hr,
+          height: 1,
+          backgroundColor: softDivider
+        },
+        html: {}
+      }),
+      [sharedProps.tagsStyles, softDivider]
+    );
     const renderHtml = (
       <RenderHTML
         debug={false}
         {...sharedProps}
         tagsStyles={mergedTagsStyles}
         baseStyle={baseStyle}
+        source={sharedProps.source}
         enableUserAgentStyles
         systemFonts={SYSTEM_FONTS}
-        remoteLoadingView={() => <UIDisplayLoadingAtom />}
-        triggerTREInvalidationPropNames={['baseStyle', 'tagsStyles']}
+        remoteLoadingView={renderRemoteLoadingView}
       />
     );
     return <View style={style}>{renderHtml}</View>;

diff --git a/apps/demo/src/components/screens/HomeDrawerScreen/index.tsx b/apps/demo/src/components/screens/HomeDrawerScreen/index.tsx
@@ -18,6 +18,7 @@ import {
   resourceRoutesIndex
 } from '../../../nav-model';
 import imagesMap from '../../../imagesMap';
+import { memo } from 'react';
 
 interface ResourceRouteNav extends ResourceRouteDefinition {
   component: React.ComponentType<any>;
@@ -51,27 +52,30 @@ const groups: Array<GroupDefinition> = Object.entries(specsByGroups).map(
     return {
       group: groupName,
       groupLabel: groupName,
-      routes: pages.map<ResourceRouteNav>((page, pageIndex) => ({
-        header: () => null,
-        component: function Page() {
-          const prevPage = pages[pageIndex - 1] || prevGroupLastPage || null;
-          const nextPage = pages[pageIndex + 1] || nextGroupFirstPage || null;
-          return (
-            <ArticleTemplate
-              title={page.title}
-              groupLabel={groupName}
-              description={page.description}
-              prevPage={prevPage}
-              nextPage={nextPage}
-              imageSource={imagesMap[page.id]}>
-              {React.createElement(page.component)}
-            </ArticleTemplate>
-          );
-        },
-        iconName: page.iconName as any,
-        name: `${groupName as PageGroup}-${page.id}` as const,
-        title: page.title
-      }))
+      routes: pages.map<ResourceRouteNav>((page, pageIndex) => {
+        return {
+          header: () => null,
+          component: function Page() {
+            const Content = memo(page.component);
+            const prevPage = pages[pageIndex - 1] || prevGroupLastPage || null;
+            const nextPage = pages[pageIndex + 1] || nextGroupFirstPage || null;
+            return (
+              <ArticleTemplate
+                title={page.title}
+                groupLabel={groupName}
+                description={page.description}
+                prevPage={prevPage}
+                nextPage={nextPage}
+                imageSource={imagesMap[page.id]}>
+                {React.createElement(Content)}
+              </ArticleTemplate>
+            );
+          },
+          iconName: page.iconName as any,
+          name: `${groupName as PageGroup}-${page.id}` as const,
+          title: page.title
+        };
+      })
     };
   }
 );

diff --git a/doc-tools/doc-pages/src/pages/PageFAQ.tsx b/doc-tools/doc-pages/src/pages/PageFAQ.tsx
@@ -104,15 +104,6 @@ export default function PageFAQ() {
         </Section>
       </Chapter>
       <Chapter title="Troubleshooting">
-        <Section title="Some props such as styling props don't cause a re-render, what's wrong?">
-          <Paragraph>
-            Props for the <RefRenderHTMLExport name="TRenderEngineConfig" />{' '}
-            component such as styling props are "cold", because a rebuild of the
-            engine is costly. To circumvent the issue, you can whitelist props
-            which should be reactive via{' '}
-            <RefRenderHtmlProp name="triggerTREInvalidationPropNames" /> prop.
-          </Paragraph>
-        </Section>
         <Section title="Custom font families don't work, what's happening?">
           <Paragraph>
             You must register fonts available in your app with{' '}

diff --git a/packages/render-html/jest/setup.js b/packages/render-html/jest/setup.js
@@ -1,3 +1,10 @@
 global.__DEV__ = true;
 
+global.performance = {
+  now() {
+    const [seconds, nano] = process.hrtime();
+    return seconds * 1000000 + nano / 1000;
+  }
+};
+
 console.warn = () => {};
diff --git a/packages/render-html/src/RenderHTML.tsx b/packages/render-html/src/RenderHTML.tsx
@@ -26,12 +26,12 @@ export default function RenderHTML(props: RenderHTMLProps): ReactElement {
     onTTreeChange,
     onDocumentMetadataLoaded,
     contentWidth,
-    ...config
+    ...otherProps
   } = props;
   return (
     <RenderHTMLDebug {...props}>
-      <TRenderEngineProvider {...props}>
-        <RenderHTMLConfigProvider {...config}>
+      <TRenderEngineProvider {...otherProps}>
+        <RenderHTMLConfigProvider {...otherProps}>
           {React.createElement(RenderHTMLSource, {
             source,
             onHTMLLoaded,

diff --git a/packages/render-html/src/RenderHTMLConfigProvider.tsx b/packages/render-html/src/RenderHTMLConfigProvider.tsx
@@ -12,6 +12,7 @@ import sourceLoaderContext, {
 } from './context/sourceLoaderContext';
 import RenderRegistryProvider from './context/RenderRegistryProvider';
 import { useAmbientTRenderEngine } from './TRenderEngineProvider';
+import useProfiler from './hooks/useProfiler';
 
 const childrenRendererContext = {
   TChildrenRenderer,
@@ -53,13 +54,14 @@ export default function RenderHTMLConfigProvider(
     ...sharedProps
   } = props;
   const engine = useAmbientTRenderEngine();
-  const sourceLoaderConfig = useMemo(
-    () => ({
+  const profile = useProfiler({ prop: 'remoteErrorView or remoteLoadingView' });
+  const sourceLoaderConfig = useMemo(() => {
+    __DEV__ && profile();
+    return {
       remoteErrorView: remoteErrorView || defaultRenderError,
       remoteLoadingView: remoteLoadingView || defaultRenderLoading
-    }),
-    [remoteErrorView, remoteLoadingView]
-  );
+    };
+  }, [remoteErrorView, remoteLoadingView, profile]);
   return (
     <RenderRegistryProvider
       renderers={renderers}

diff --git a/packages/render-html/src/RenderHTMLDebug.tsx b/packages/render-html/src/RenderHTMLDebug.tsx
@@ -42,6 +42,9 @@ const RenderHTMLDebug = function RenderHTMLDebug(
     if ('computeImagesMaxWidth' in props) {
       console.warn(debugMessage.outdatedComputeImagesMaxWidth);
     }
+    if ('triggerTREInvalidationPropNames' in props) {
+      console.warn(debugMessage.outdatedTriggerTREInvalidation);
+    }
     if (Array.isArray(props.allowedStyles)) {
       props.allowedStyles.forEach((s) => {
         if (s.indexOf('-') > -1) {

diff --git a/packages/render-html/src/RenderHTMLSource.tsx b/packages/render-html/src/RenderHTMLSource.tsx
@@ -18,6 +18,7 @@ import SourceLoaderDom from './SourceLoaderDom';
 import debugMessage from './debugMessages';
 import contentWidthContext from './context/contentWidthContext';
 import isDomSource from './helpers/isDomSource';
+import useProfiler from './hooks/useProfiler';
 
 export type RenderHTMLSourcePropTypes = Record<
   keyof RenderHTMLSourceProps,
@@ -61,6 +62,7 @@ function RawSourceLoader({
   ...props
 }: SourceLoaderProps): ReactElement | null {
   if (isEmptySource(source)) {
+    /* istanbul ignore next */
     if (__DEV__) {
       console.warn(debugMessage.noSource);
     }
@@ -92,13 +94,16 @@ const RenderHTMLSource = memo(
     contentWidth,
     ...props
   }: RenderHTMLSourceProps) {
-    const ttreeEvents: TTreeEvents = useMemo(
-      () => ({
+    const profile = useProfiler({
+      prop: 'onDocumentMetadataLoaded or onTTreeChange'
+    });
+    const ttreeEvents: TTreeEvents = useMemo(() => {
+      __DEV__ && profile();
+      return {
         onDocumentMetadataLoaded,
         onTTreeChange
-      }),
-      [onDocumentMetadataLoaded, onTTreeChange]
-    );
+      };
+    }, [onDocumentMetadataLoaded, onTTreeChange, profile]);
     if (__DEV__) {
       if (!(typeof contentWidth === 'number')) {
         console.warn(debugMessage.contentWidth);

diff --git a/packages/render-html/src/TRenderEngineProvider.tsx b/packages/render-html/src/TRenderEngineProvider.tsx
@@ -39,8 +39,7 @@ export const tRenderEngineProviderPropTypes: Record<
   setMarkersForTNode: PropTypes.func,
   dangerouslyDisableHoisting: PropTypes.bool,
   dangerouslyDisableWhitespaceCollapsing: PropTypes.bool,
-  selectDomRoot: PropTypes.func,
-  triggerTREInvalidationPropNames: PropTypes.arrayOf(PropTypes.string)
+  selectDomRoot: PropTypes.func
 };
 
 /**
@@ -67,8 +66,7 @@ export const defaultTRenderEngineProviderProps: TRenderEngineConfig = {
   enableCSSInlineProcessing: true,
   customHTMLElementModels: {},
   fallbackFonts: defaultFallbackFonts,
-  systemFonts: defaultSystemFonts,
-  triggerTREInvalidationPropNames: []
+  systemFonts: defaultSystemFonts
 };
 
 /**

diff --git a/packages/render-html/src/__tests__/component.render-html-debug.test.tsx b/packages/render-html/src/__tests__/component.render-html-debug.test.tsx
@@ -55,13 +55,18 @@ describe('RenderHTMLDebug', () => {
     () => {},
     'outdatedComputeImagesMaxWidth'
   );
+  createOutdatedPropTest(
+    'triggerTREInvalidationPropNames',
+    () => {},
+    'outdatedTriggerTREInvalidation'
+  );
   it('should warn of allowedStyles items with hyphens', () => {
     console.warn = jest.fn();
     render(
       //@ts-ignore
       React.createElement(RenderHTMLDebug, {
         //@ts-expect-error
-        allowedStyles: ['hello-world'],
+        allowedStyles: ['hello-world', 'color'],
         debug: false,
         contentWidth: 10
       })
@@ -75,7 +80,7 @@ describe('RenderHTMLDebug', () => {
       //@ts-ignore
       React.createElement(RenderHTMLDebug, {
         //@ts-expect-error
-        ignoredStyles: ['hello-world'],
+        ignoredStyles: ['hello-world', 'color'],
         debug: false,
         contentWidth: 10
       })

diff --git a/packages/render-html/src/__tests__/component.render-html.test.tsx b/packages/render-html/src/__tests__/component.render-html.test.tsx
@@ -3,7 +3,10 @@ import { act, render, waitFor } from 'react-native-testing-library';
 import RenderHTML from '../RenderHTML';
 import ImgTag from '../elements/IMGElement';
 import TTextRenderer from '../TTextRenderer';
-import { CustomTextualRenderer } from '../render/render-types';
+import {
+  CustomBlockRenderer,
+  CustomTextualRenderer
+} from '../render/render-types';
 import {
   defaultHTMLElementModels,
   HTMLContentModel
@@ -581,5 +584,27 @@ describe('RenderHTML', () => {
       );
       await waitFor(() => UNSAFE_getByType(Text));
     });
+    it('should warn when using the default WebView component', () => {
+      const ImageRenderer: CustomBlockRenderer = jest.fn(function SpanRenderer({
+        sharedProps: { WebView }
+      }) {
+        return <WebView />;
+      });
+      console.warn = jest.fn();
+      render(
+        <RenderHTML
+          source={{
+            html: '<img />'
+          }}
+          debug={false}
+          renderers={{
+            img: ImageRenderer
+          }}
+          contentWidth={100}
+          enableExperimentalMarginCollapsing={false}
+        />
+      );
+      expect(console.warn).toHaveBeenCalled();
+    });
   });
 });
diff --git a/packages/render-html/src/__tests__/regression.343.tags-styles-re-render.test.tsx b/packages/render-html/src/__tests__/regression.343.tags-styles-re-render.test.tsx
@@ -19,21 +19,19 @@ describe('RenderHTML component', () => {
   const tagsStylesInstance2 = {
     a: letterSpacing3
   };
-  it('should pass regression #343 regarding tagsStyles prop (requires triggerTREInvalidationPropNames)', () => {
+  it('should pass regression #343 regarding tagsStyles prop', () => {
     const { getByText, update } = render(
       <RenderHTML
         debug={false}
         source={{ html: '<a>hello world</a>' }}
         tagsStyles={tagsStylesInstance1}
-        triggerTREInvalidationPropNames={['tagsStyles']}
       />
     );
     update(
       <RenderHTML
         debug={false}
         source={{ html: '<a>hello world</a>' }}
         tagsStyles={tagsStylesInstance2}
-        triggerTREInvalidationPropNames={['tagsStyles']}
       />
     );
     const text = getByText('hello world');