react-component / tween-one

Animate One React Element
https://tween-one.vercel.app/
MIT License
380 stars 30 forks source link

null is not an object (evaluating 'this.target.ownerSVGElement') #32

Closed mb8z closed 6 years ago

mb8z commented 6 years ago

Hi!

First of all - awesome package. Make apps look really fancy with all those animations!

I've started receiving the following error:

null is not an object (evaluating 'this.target.ownerSVGElement')

Tried to debug it for a long time. It just happens when I load the page that uses this package, but not always (approx. 10 out of 100 cases). Unfortunately that's all that I currently have.

Any ideas why it might be happening and what can I or we do to stop it?

edit I've seen that it was the case of PR27. What was the reason it was not an acceptable solution?

jljsj33 commented 6 years ago

Can you tell me how you got the wrong report?

jljsj33 commented 6 years ago

Check it out, there are 2 kinds of situations will come out this error:

TweenOneGroup

I once received the same error in the route, if there is uncertainty in the children, it is recommended to add a layer of div.

<Router history={history}>
  <Route render={({ location }) => {
    const to = '/list';
    const key = location.pathname === '/' ? to : location.pathname;
    return (
      <TweenOneGroup
        appear={false}
        enter={{ y: 30, opacity: 0, type: 'from' }}
        leave={{ y: -30, opacity: 0 }}
      >
+        <div key={key} id={key}>
          <Switch location={location}>
            <Route path="/new" component={New} key="new" />
            <Route path="/list" component={List} key="list" />
            <Redirect from="/" exact to={to} />
          </Switch>
+        </div>
      </TweenOneGroup>);
  }} />
</Router>

component return is null ref #27

or simple demo TweenOne component=""

function Div() {
  return (null);
}

function Demo() {
  return (<TweenOne
    animation={{ x: 300 }}
-    component="" // remove component="" 
  >
    <Div>test</Div>
  </TweenOne>);
}

or

function Div() {
  return (null);
}

function Demo() {
  return (<TweenOne
    animation={{ x: 300 }}
-    component={Div}
  >
+  <Div>
      test
+  </Div>
  </TweenOne>);
} 

or

function Div() {
-  return (null);
+  return <div>{null}</div>
}

function Demo() {
  return (<TweenOne
    animation={{ x: 300 }}
    component={Div}
  >
    test
  </TweenOne>);
} 
mb8z commented 6 years ago

The main problem with this issue is that I do not exactly know how to reproduce it. When I enter a page when there is a TweenOneGroup used I get it once in a while, not like always.

I have appear set to false as in your examples. Also, the children are wrapped in <div> tags, but without using thecomponent` prop.

What's more is that I never experienced it locally, but only in production environment.

The records that I put inside the TweenOneGroup are fetched on componentDidMount, so at the beginning there are none, but the response is really quick and the records appear in a matter of ms.

Edit I am using TweenOneGroup in my own component wrapper like this:

const Animatable = ({
  children,
}) => (
    <TweenOneGroup
      enter={/* my enter anim */}
      leave={/* my leave anim */}
      appear={false}
      className="animation-from-right"
    >
      {children}
    </TweenOneGroup>
  );

I now replaced children with:

{React.Children.map(children, (child, i) => 
   child && React.cloneElement(child);
)}

Based on your assumptions it might be helpful as we will have a check if there is a child.

mb8z commented 6 years ago

Unfortunately it happened again. And it is quite impossible to debug as it happens raindomly and very rarely.

It might be this line that is crucial: https://github.com/react-component/tween-one/blob/6c53c14d5d67dd587d717bafde502595713cb071/src/TweenOne.jsx#L31

In some cases it might return null. In this case the following line would also pass null to Tween method: https://github.com/react-component/tween-one/blob/6c53c14d5d67dd587d717bafde502595713cb071/src/TweenOne.jsx#L171

Do you think it is good idea to add a check in start method to check if this.dom was properly set? Thanks for your time! :tada:

jljsj33 commented 6 years ago

I thought about it this, but need to consider that componentDidUpdate is not null.

jljsj33 commented 6 years ago

1.8.0

mb8z commented 6 years ago

That was quite a lot of changes for just this one line rarely messing up. Great work :tada: And BIG BIG thanks :)!

jljsj33 commented 6 years ago

1.8.1... publish failure