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

Disconnected Events #678

Closed
pemrouz opened this issue Oct 31, 2017 · 13 comments
Closed

Disconnected Events #678

pemrouz opened this issue Oct 31, 2017 · 13 comments

Comments

@pemrouz
Copy link

pemrouz commented Oct 31, 2017

Would it be possible to emit disconnected events on a node? I'd like to be able to do something like the following:

class MyComponent extends HTMLElement {
 
  connectedCallback() {
    framework
      .subscribe(['some', 'data'])
      .map(([some, data]) => node.state = { some, data ...node.state })
      .map(node.draw)
      .until(node.once('disconnected'))
   }

 async render(node, { some, data }) {
  // update component here
 }
}

(node.on/once is just simply creating a stream/promise of event(s) from the DOM element)

This can obviously done with some wrapper but it would be nice if it didn't need that. Also I'm not sure if this can be done for all elements rather than just custom elements?

Related: Don't Unsubscribe

@treshugart
Copy link

treshugart commented Oct 31, 2017

Would you be able to use the disconnectedCallback() to trigger the disconnected event?

@rniwa
Copy link
Collaborator

rniwa commented Oct 31, 2017

Async event? We certainly don't want to add a synchronous event. That kind of DOM mutation events is exactly what we're trying to get rid of due to a high rate of security vulnerabilities caused by them and performance problems they cause.

@pemrouz
Copy link
Author

pemrouz commented Oct 31, 2017

Would you be able to use the disconnectedCallback() to trigger the disconnected event?

Yep, you can. But it would be nice not to have to imperatively unsubscribe (see the article) - plus that's only internal to the component. The framework already does this btw (emit an event in disconnectedCallback) but it would be nice if it didn't have to.

Async event? We certainly don't want to add a synchronous event. That kind of DOM mutation events is exactly what we're trying to get rid of due to a high rate of security vulnerabilities caused by them and performance problems they cause.

Given disconnectedCallback is already there, wouldn't this just be some sugar? Or are you suggesting to get rid of disconnectedCallback?

@rniwa
Copy link
Collaborator

rniwa commented Oct 31, 2017

Given disconnectedCallback is already there, wouldn't this just be some sugar? Or are you suggesting to get rid of disconnectedCallback?

disconnectedCallback happens as a custom element reactions. We shouldn't fire events at this timing since that results in multiple listeners racing to undo one another's changes. At the earlier we could fire would be the end of the micro-task timing which would be much later than when disconnectedCallback happens.

@pemrouz
Copy link
Author

pemrouz commented Nov 2, 2017

disconnectedCallback happens as a custom element reactions. We shouldn't fire events at this timing since that results in multiple listeners racing to undo one another's changes. At the earlier we could fire would be the end of the micro-task timing which would be much later than when disconnectedCallback happens.

👍. The exact timing is not so important for me.

In terms of spec/implementation, would this be done more generically for all custom element reactions?

@rniwa
Copy link
Collaborator

rniwa commented Nov 2, 2017

If you're fine with the micro-task timing, you should just use https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver. It's implemented in all major browsers now.

In terms of spec/implementation, would this be done more generically for all custom element reactions?

No. We'd have to add it for each custom element reactions based on use cases.

@pemrouz
Copy link
Author

pemrouz commented Nov 3, 2017

If you're fine with the micro-task timing, you should just use https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver. It's implemented in all major browsers now.

How would you use that exactly in this case, short of watching the entire document?

No. We'd have to add it for each custom element reactions based on use cases.

Just disconnected would be great. Doesn't really make sense to emit connected since no-one will/can be listening, it's commonly used for housekeeping, and it's very painful to try replicate with MutationObservers (i.e. compared to attributes where you just need to observe a single node).

@rniwa
Copy link
Collaborator

rniwa commented Nov 3, 2017

How would you use that exactly in this case, short of watching the entire document?

Just create a new mutation observer for each node you have to monitor?

@pemrouz
Copy link
Author

pemrouz commented Nov 3, 2017

Just create a new mutation observer for each node you have to monitor?

What if the parent of the node you're observing is detached?

This is currently extremely painful to do in userland (and even then it's buggy due to batching)

@rniwa
Copy link
Collaborator

rniwa commented Nov 3, 2017

Oh I see. We probably need to add new options to observe disconnected & connected events to MutationObserver.

@pemrouz
Copy link
Author

pemrouz commented Nov 4, 2017

Shall I raise this on whatwg/dom then?

@rniwa
Copy link
Collaborator

rniwa commented Nov 4, 2017

Filed whatwg/dom#533.

@annevk
Copy link
Collaborator

annevk commented Feb 17, 2018

Closing this in favor of the upstream feature request referenced above.

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