Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error: Maximum update depth exceeded #12

Open
jflayhart opened this issue Sep 13, 2018 · 22 comments
Open

Error: Maximum update depth exceeded #12

jflayhart opened this issue Sep 13, 2018 · 22 comments

Comments

@jflayhart
Copy link

jflayhart commented Sep 13, 2018

I can't ever seem to get past this error:

screen shot 2018-09-13 at 6 49 53 pm

Currently the library seems unusable =/ Not sure how to guard against it within the componentDidUpdate lifecycle hook.

@modestfake
Copy link

modestfake commented Sep 18, 2018

I have the same issue. And also I see ResizeObserver loop limit exceeded in logs. I use it along with react-linkify.

<TruncateMarkup lines={3}>
  <span>
    <Linkify properties={{ target: '_blank' }}>{message}</Linkify>
  </span>
</TruncateMarkup>

@modestfake
Copy link

modestfake commented Sep 18, 2018

I've reproduced it in this codesandbox.
Change maxWidth to 300 and it works. With 200 it reproduces this issue.

And logs from sentry.io
image

@patrik-piskay
Copy link
Owner

Thanks for the report (and codesandbox ), I'll take a look.

@jflayhart any chance you can provide a repro case (codesandbox) too? Just to help verify the fix

@patrik-piskay
Copy link
Owner

Update - I found the issue and it's not a small one. It affects HOC/render prop components (that's the case for @modestfake's example, as the Mention component used is using both). I'll need more time to investigate and come up with a solution for these cases.

@modestfake
Copy link

@patrik-piskay no rush, thanks for your time, appreciate it!

@patrik-piskay
Copy link
Owner

patrik-piskay commented Dec 6, 2018

So here is what I found out...

Issue: react-truncate-markup doesn't support React components as its children. Not just HOC/render props as mentioned in #12 (comment), but all components.

Solution: tl;dr: there is none that I am aware of that works for every case, so in dev mode we'll warn if React components are used as <TruncateMarkup /> children and skip any truncation, displaying the whole content instead. <TruncateMarkup /> will work fine when using inlined DOM elements only.


I've spent a few hours trying to get React components (both classes and functions) to work. The reason it's not trivial is because of the way the truncate functionality works here. General approach is to take the whole element tree structure (props.children of the parent component) and work with that. This tree might look like this:

<TruncateMarkup>
  <div>
    Hello <span>world{'!'}</span>
  </div>
</TruncateMarkup>

↓↓↓

{
  type: 'div',
  props: {
    children: [
      'Hello',
      {
        type: 'span',
        props: {
          children: [
            'world',
            '!'
          ]
        }
      }
    ]
  }
}

which is easy to work with as we have the whole final "render" tree available, which means we can potentially end up with this result (after truncation):

<div>
  Hello <span>wo</span>...
</div>

But if we were to move a part of the tree to a separate component, it could look like this:

const Person = ({ person }) => (
  <span>{person || 'world'} </span>
);

<TruncateMarkup>
  <div>
    Hello <Person />!
  </div>
</TruncateMarkup>

↓↓↓

{
  type: 'div',
  props: {
    children: [
      'Hello',
      {
        type: 'function Person()',
        props: {}
      },
      '!'
    ]
  }
}

which hides what's getting rendered in <Person /> so we cannot use that in the current truncation algorithm. We could truncate only after Hello or after Hello <span>{person}</span>, but we'd never be able to truncate anything inside the <span>{person}</span> because it's not initially visible to us, only after React renders it into the DOM.

One idea I had to solve this was to "unwrap" the whole tree before starting any truncation work by "evaluating" these nested components:

if (typeof node.type === 'function') {
  if (// class component) {
    const instance = new node.type(node.props);
    return instance.render();
  } else {
    // function component
    return node.type(node.props);
  }
}

This actually worked and I was able to get nested components to work, until...it didn't.

Some components I used for testing used context, which this approach didn't support. Even before trying this approach I felt it's probably not the best idea to start emulating React itself by rendering the tree on our own, and when I experienced the context issues, I decided it's (probably) nothing we can solve reliably, so the best thing we could do is to officially not support it - we'll warn if React components are detected in the <TruncateMarkup /> children and will skip any truncation work, rendering the whole content instead.

<TruncateMarkup /> will continue to work fine when using inlined DOM elements as its children like in the original case:

<TruncateMarkup>
  <div>
    Hello <span>world{'!'}</span>
  </div>
</TruncateMarkup>

@modestfake
Copy link

Thank you @patrik-piskay for the detailed explanation! Really appreciate your effort and time!

Maybe you've already thought about this, so I just wondering, can you use refs? With refs you can have a full access to the DOM, rendered by React. (I'm not sure I understand how your component works, so I might be wrong, don't blame me for that 😕 ).

@patrik-piskay
Copy link
Owner

The issue with refs is that it returns actual DOM nodes instead of React elements. So instead of

{
  type: 'div',
  props: {
    children: [
      'Hello',
      {
        type: 'span',
        props: {
          children: [
            'world',
            '!'
          ]
        }
      }
    ]
  }
}

we get

 <div>Hello <span>world!</span></div>

That means we'd need to transform the returned DOM nodes back into React elements again (or any tree structure so we can truncate it and have it rendered again), which might be doable, but again not reliable. For example, I don't think events (click etc.) would work. Also, we'd lose access to any React components after the initial render (because we'd continue working with the initially returned DOM elements), so any effects happening in their lifecycle methods would be lost.

It's probably best to keep it simple for now and use DOM elements only. For the future, who knows, maybe with Hooks there will be something we can do to support React components, I'll keep my eyes on it.

@modestfake
Copy link

Got it! Thanks again!

@patrik-piskay
Copy link
Owner

I just published a new (major) version that checks for React components inside the children, and if there are any, it'll skip the truncation.

Closing this issue as that (React components used as children) looks to be the reason for the originally reported error.

@agduyaguit
Copy link

This issue is still occurring when width is less 300px.

@patrik-piskay
Copy link
Owner

Hey @agduyaguit, can you post a link to codesanbox with a repro case? Thanks!

@agduyaguit
Copy link

agduyaguit commented Apr 19, 2019

hi @patrik-piskay ,

Here is the test data I used to replicate the issue:

Screen Shot 2019-04-19 at 16 58 04

            <TruncateMarkup lines={3}>
                <div style={{width: '150px'}}>
                  <div style={{display: 'inline'}}>
                    <span>hello</span>
                  </div>
                  <div style={{display: 'inline'}}>
                    <span>hello</span>
                  </div>
                  <div style={{display: 'inline'}}>
                    <img src='someimage' alt=''></img>
                    <span>hello</span>
                  </div>
                  <div style={{display: 'inline'}}>
                    <img src='someimage' alt=''></img>
                    <span>hello</span>
                  </div>
                  <div style={{display: 'inline'}}> 
                    <img src='someimage' alt=''></img>
                    <span>hello</span>
                  </div>
                  <div style={{display: 'inline'}}> 
                    <img src='someimage' alt=''></img>
                    <span>hello</span>
                  </div>
                  <div style={{display: 'inline'}}> 
                    <img src='someimage' alt=''></img>
                    <span>hello</span>
                  </div>
                  <div style={{display: 'inline'}}> 
                    <img src='someimage' alt=''></img>
                    <span>hello</span>
                  </div>
                  <div style={{display: 'inline'}}> 
                    <img src='https://material.io/tools/icons/static/icons/baseline-donut_large-24px.svg' alt=''></img>
                    <span>hello</span>
                  </div>
                  <div style={{display: 'inline'}}> 
                    <img src='https://material.io/tools/icons/static/icons/baseline-donut_large-24px.svg' alt=''></img>
                    <span>hello</span>
                  </div>
                  <div style={{display: 'inline'}}> 
                    <img src='someimage' alt=''></img>
                    <span>hello</span>
                  </div>
                  <div style={{display: 'inline'}}> 
                    <img src='someimage' alt=''></img>
                    <span>hello</span>
                  </div>
                </div>
              </TruncateMarkup>
            </div>

@patrik-piskay patrik-piskay reopened this Apr 19, 2019
@nvuhung
Copy link

nvuhung commented May 28, 2019

I am facing same problem. Any update for this?

@patrik-piskay
Copy link
Owner

Can you post your case example on codesanbox @nvuhung?

@nvuhung
Copy link

nvuhung commented May 29, 2019

Can you post your case example on codesanbox @nvuhung?

My code is same as @agduyaguit

@drewlustro
Copy link

I am also experiencing this issue; the package is not usable because it crashes the whole app.

Is there a way to throttle the resize event?

I'm passing simple DOM HTML into the component (not complex React components).

@drewlustro
Copy link

Workaround

cc @nvuhung @agduyaguit @modestfake @jflayhart

I developed a workaround for this issue until it is fixed. This module is overall great, but clearly this problem is complex.

Using an error boundary with fallbacks, we can render the unabridged markup instead of crashing.

 

💡 ErrorBoundary component

import React from 'react';

interface State {
  hasError: boolean;
}

interface Props {
  children: React.ReactNode;
  className?: string;
  fallback?: React.ReactNode;
  fallbackHtml?: string;
  name: string;
}

class ErrorBoundary extends React.Component<Props, State> {
  state: State = {
    hasError: false,
  };

  componentDidCatch(error: any, info: any) {
    const { name } = this.props;
    this.setState({ hasError: true });

    log(
      `%cCaught "${name}" ErrorBoundary`,
      'color: #c0392b; font-weight: 600;',
    );
    log('%s', error);
    log('%j', info);
  }

  render() {
    const { hasError } = this.state;
    const { children, fallback, fallbackHtml } = this.props;

    if (hasError) {
      if (fallbackHtml) {
        return <div dangerouslySetInnerHTML={{ __html: fallbackHtml }} />;
      }

      if (fallback) {
        return fallback;
      }

      return (
        <div>
          An unexpected error occurred.
          <br />
          <br />
          <a href=".">Reload page</a>.
        </div>
      );
    }

    return children;
  }
}

export default ErrorBoundary;

 

Apply boundary to <TruncateMarkup>

import convert from 'htmr';

// ...

<ErrorBoundary
  name="Profile TruncateMarkup"
  fallbackHtml={profileRawHtml}
>
  <TruncateMarkup lines={4} ellipsis={readMore} tokenize="words">
    <div>{convert(profileRawHtml)}</div>
  </TruncateMarkup>
</ErrorBoundary>

Result when error boundary is hit (app does not crash)

Screen Shot 2019-05-31 at 1 13 31 PM

@patrik-piskay
Copy link
Owner

Hey @drewlustro, can you post a codesanbox of your specific issue? we need as many failing cases as possible because, as you said, it's a complex problem and sometimes it's the markup that's passed into the component that's causing it, like <img /> tags in (for now the only) example above. That helps with understanding the issue more.

Specifically, I'd be interested to see what convert(profileRawHtml) produces. If you can make a repro case, that would be great!

@svobik7
Copy link

svobik7 commented Jan 9, 2020

Is there any progress? I am facing the same issue.

@patrik-piskay
Copy link
Owner

Hi @svobik7, any chance you can provide a repro case (using codesanbox or some other, similar online tool)? It'll be easier to spot the issue is that way.

@svobik7
Copy link

svobik7 commented Jan 10, 2020

Hi @patrik-piskay, unfortunately my repo is private. But I discovered that error happend only when content wrapper width is < 300px. Maybe it will help.

As I was pushed to solve this problem I had to use another library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants