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

"Uncaught TypeError: Cannot read property 'update' of null" when calling set in an event after destroy #744

Closed
btakita opened this issue Aug 3, 2017 · 8 comments

Comments

@btakita
Copy link
Contributor

btakita commented Aug 3, 2017

This manifests when using an evented object that has a listener that destroys the Component & a subsequent listener that sets a value on the Component.

https://svelte.technology/repl?version=1.27.0&gist=16c53dce350c0a67c6bb1ff8db61ab88

@snuggs
Copy link

snuggs commented Aug 4, 2017

@btakita what should the intended behaviour be? And what's up man lol.

@Rich-Harris
Copy link
Member

Would a dev mode error like this solve the problem?

You cannot call .set(...) on a component that has been destroyed

@btakita
Copy link
Contributor Author

btakita commented Aug 4, 2017

I'd rather it not throw an error.

Right now, I shimmed a method that checks to see if the component is "active" before calling set.

export function set__svelte(C, ...rest) {
  return C._fragment ? C.set(...rest) : C
}
export const set = set__svelte

@Rich-Harris
Copy link
Member

What's the situation in which set is being called after destroy? Just wondering if there's a different way of getting at this, without adding a this._destroyed check for each set call (which certainly isn't the worst thing in the world, but good to avoid it if it's masking a separate problem)

@btakita
Copy link
Contributor Author

btakita commented Aug 4, 2017

It's difficult to avoid this issue using events that are ordered by definition, as previously defined events may triggers destroy before a later defined event triggers set.

Here are three areas where the logic could be handled:

  • Event handler ordering - imposes difficult & otherwise unnecessary constraints on the programmer
  • Guard clause in the event handler - Which is what I'm doing right now. It creates an otherwise unnecessary helper function, though it's relatively painless & could be a useful "seam" later on
  • console.warn
  • this._destroyed guard clause in svelte

I'm ok with console.warn, even though it would still encourage me to have a helper function. Letting it throw an error would require debugging for somebody unfamiliar with the problem. Maybe put a link to this ticket, or a section in the Readme to resolve?

I would slightly prefer the this._destroyed guard but now that there's a helper function, it's not that big of a deal.

@TehShrike
Copy link
Member

I debounce some method calls to the next animation frame, and in those methods I've been forced to check for this._destroyed before continuing.

@Rich-Harris
Copy link
Member

@TehShrike @btakita #755 fixes this by replacing the set and destroy methods with noop when you first call destroy. Does that seem like a good solution? I ask because of this...

I've been forced to check for this._destroyed before continuing

...which will no longer be possible, because (I think) there's no longer a need for the _destroyed flag.

@TehShrike
Copy link
Member

TehShrike commented Aug 6, 2017

that sounds like the perfect solution.

Could all user-defined methods also be replaced with noop on destroy?

Rich-Harris added a commit that referenced this issue Aug 9, 2017
handle set after destroy, and move destroy into shared helpers
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

4 participants