meliorence / react-native-render-html

iOS/Android pure javascript react-native component that renders your HTML into 100% native views
https://meliorence.github.io/react-native-render-html/
BSD 2-Clause "Simplified" License
3.48k stars 589 forks source link

Infinite re-rendering using provided custom rendering code from docs #580

Open CDBridger opened 2 years ago

CDBridger commented 2 years ago

Decision Table

Good Faith Declaration

Description

When using this code

import {
    useInternalRenderer,
    InternalRendererProps,
    IMGElementProps,
    IMGElementContainer,
    useIMGElementProps,
    useIMGElementState,
    IMGElementContentSuccess,
    IMGElementContentLoading,
    IMGElementContentError
} from 'react-native-render-html';

export const HtmlImageRenderer: React.FC<InternalRendererProps<any>> = props => {
    // const { Renderer, rendererProps } = useInternalRenderer('img', props);
    const imgElementProps = useIMGElementProps(props);
    const modifiedSource = getModifiedSource(imgElementProps);
    const state = useIMGElementState(modifiedSource);
    let content: ReactNode = false;
    if (state.type === 'error') {
        content = React.createElement(IMGElementContentError, state);
    } else if (state.type === 'loading') {
        content = React.createElement(IMGElementContentLoading, state);
    } else {
        content = React.createElement(IMGElementContentSuccess, state);
    }
    return (
        <IMGElementContainer onPress={imgElementProps.onPress} style={state.containerStyle}>
            {content}
        </IMGElementContainer>
    );
};

export const HTMLRenderers = {
    img: HtmlImageRenderer,
};

The component constantly re renders if the URL fails to load. You can see the output of WhyDidYouRender.js here and this snippet is passed to my console continuously.

HtmlImageRenderer
​ {HtmlImageRenderer: {…}} 'Re-rendered because of hook changes:'
​ [hook useState result]
​ different objects that are equal by value. (more info at http://bit.ly/wdyr3)
​ {"prev ": Error: Failed to load https://--SNIP--.dev/assets/Uploads/2022-08/Balckstone-web.jpeg…} '!==' {"next ": Error: Failed to load https://--SNIP--.dev/assets/Uploads/2022-08/Balckstone-web.jpeg…}
​ {"For detailed diff, right click the following fn, save as global, and run: ": ƒ}

I don't get this if I use the userInternalRenderer route, but the reason I was changing to this style was So I could get at the default Props for the image component used to load a placeholder image if the image fails to load like in this example. Obviously I can't have the component infinitely re rendering though.

React Native Information

System:
    OS: macOS 12.5.1
    CPU: (8) arm64 Apple M1 Pro
    Memory: 89.41 MB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 18.7.0 - /opt/homebrew/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 8.17.0 - /opt/homebrew/bin/npm
    Watchman: Not Found
  Managers:
    CocoaPods: 1.11.3 - /opt/homebrew/bin/pod
  SDKs:
    iOS SDK:
      Platforms: DriverKit 21.4, iOS 15.5, macOS 12.3, tvOS 15.4, watchOS 8.5
    Android SDK:
      API Levels: 31, 33
      Build Tools: 30.0.3, 31.0.0, 33.0.0
      System Images: android-33 | Google APIs ARM 64 v8a
      Android NDK: Not Found
  IDEs:
    Android Studio: 2021.2 AI-212.5712.43.2112.8815526
    Xcode: 13.4.1/13F100 - /usr/bin/xcodebuild
  Languages:
    Java: 18.0.2 - /usr/bin/javac
  npmPackages:
    @react-native-community/cli: Not Found
    react: 18.0.0 => 18.0.0 
    react-native: ^0.69.4 => 0.69.4 
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

RNRH Version

6.3.4

Tested Platforms

Reproduction Platforms

Minimal, Reproducible Example

See code above

Additional Notes

No response

CDBridger commented 2 years ago

I believe this is a patch that fixes the issue for me. I'll have to do further testing

diff --git a/node_modules/react-native-render-html/src/elements/useIMGElementState.ts b/node_modules/react-native-render-html/src/elements/useIMGElementState.ts
index 6590d21..834499d 100644
--- a/node_modules/react-native-render-html/src/elements/useIMGElementState.ts
+++ b/node_modules/react-native-render-html/src/elements/useIMGElementState.ts
@@ -78,7 +78,7 @@ function useFetchedNaturalDimensions(props: {
       if (source.uri && !hasCachedDimensions) {
         getImageSizeAsync({ uri: source.uri, headers: source.headers })
           .then((dimensions) => !cancelled && onNaturalDimensions(dimensions))
-          .catch((e) => !cancelled && onError(e || {}));
+          .catch((e) => !cancelled && onError(e?.message || {}));
         return () => {
           cancelled = true;
         };

EDIT: Nope, nvm doesn't work. this patch then jsut solves the error causing infinite re-rendering, but exposes infinite re-rendering due to dimension object causing infinite re-rendering

CDBridger commented 2 years ago

Ok this new patch seems to have done the trick.

diff --git a/node_modules/react-native-render-html/src/elements/useIMGElementState.ts b/node_modules/react-native-render-html/src/elements/useIMGElementState.ts
index 6590d21..f4d6e16 100644
--- a/node_modules/react-native-render-html/src/elements/useIMGElementState.ts
+++ b/node_modules/react-native-render-html/src/elements/useIMGElementState.ts
@@ -41,24 +41,33 @@ function useImageNaturalDimensions<P extends UseIMGElementStateProps>(props: {
       ? ImageDimensions
       : ImageDimensions | null
   >((cachedNaturalDimensions as any) || null);
+  const [naturalHeight, setNaturalHeight] = useState<number| null>();
+  const [naturalWidth, setNaturalWidth] = useState<number| null>();
+
   const { width: cachedNaturalWidth, height: cachedNaturalHeight } =
     cachedNaturalDimensions || {};
   const [error, setError] = useState<null | Error>(null);
   useEffect(
     function resetOnURIChange() {
-      setNaturalDimensions(
-        (cachedNaturalWidth != null && cachedNaturalHeight != null
-          ? { width: cachedNaturalWidth, height: cachedNaturalHeight }
-          : null) as any
-      );
+      // setNaturalDimensions(
+      //   (cachedNaturalWidth != null && cachedNaturalHeight != null
+      //     ? { width: cachedNaturalWidth, height: cachedNaturalHeight }
+      //     : null) as any
+      // );
+      setNaturalHeight(cachedNaturalHeight)
+      setNaturalWidth(cachedNaturalWidth)
       setError(null);
     },
     [cachedNaturalHeight, cachedNaturalWidth, source.uri]
   );
   return {
     onNaturalDimensions: setNaturalDimensions,
+    onNaturalHeight: setNaturalHeight,
+    onNaturalWidth: setNaturalWidth,
     onError: setError,
     naturalDimensions,
+    naturalHeight,
+    naturalWidth,
     error
   };
 }
@@ -69,7 +78,7 @@ function useFetchedNaturalDimensions(props: {
   specifiedDimensions: IncompleteImageDimensions;
 }) {
   const { source, cachedNaturalDimensions } = props;
-  const { error, naturalDimensions, onError, onNaturalDimensions } =
+  const { error, onError, onNaturalDimensions, onNaturalHeight, onNaturalWidth, naturalHeight, naturalWidth } =
     useImageNaturalDimensions(props);
   const hasCachedDimensions = !!cachedNaturalDimensions;
   useEffect(
@@ -77,8 +86,8 @@ function useFetchedNaturalDimensions(props: {
       let cancelled = false;
       if (source.uri && !hasCachedDimensions) {
         getImageSizeAsync({ uri: source.uri, headers: source.headers })
-          .then((dimensions) => !cancelled && onNaturalDimensions(dimensions))
-          .catch((e) => !cancelled && onError(e || {}));
+          .then((dimensions) => {  if (!cancelled) { onNaturalHeight(dimensions.height); onNaturalWidth(dimensions.width) }})
+          .catch((e) => !cancelled && onError(e?.message || {}));
         return () => {
           cancelled = true;
         };
@@ -87,16 +96,19 @@ function useFetchedNaturalDimensions(props: {
     [
       source.uri,
       source.headers,
-      onNaturalDimensions,
+      onNaturalHeight,
+      onNaturalWidth,
       onError,
       hasCachedDimensions
     ]
   );
   return {
-    naturalDimensions,
+    naturalHeight,
+    naturalWidth,
     error,
     onError,
-    onNaturalDimensions
+    onNaturalHeight,
+    onNaturalWidth
   };
 }

@@ -126,14 +138,14 @@ export default function useIMGElementState(
     specifiedDimensions,
     source
   });
-  const { naturalDimensions, onError, error } = useFetchedNaturalDimensions({
+  const { naturalHeight, naturalWidth, onError, error } = useFetchedNaturalDimensions({
     source: nomalizedSource,
     specifiedDimensions,
     cachedNaturalDimensions
   });
   const concreteDimensions = useImageConcreteDimensions({
     flatStyle,
-    naturalDimensions,
+    naturalDimensions: naturalHeight && naturalWidth ? {width: naturalWidth, height: naturalHeight} : null,
     specifiedDimensions,
     computeMaxWidth,
     contentWidth

I guess it's because even though the dimensions were the same, object equality wasn't, causing a rerender to trigger somewhere. Same with the error

jsamr commented 2 years ago

@CDBridger Thanks for the thorough investigation, I'll try to provide a fix soon