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

RFC for node-reactions #11

Closed
wants to merge 7 commits into from
Closed

Conversation

ravijayaramappa
Copy link
Contributor

@ravijayaramappa ravijayaramappa commented Jul 19, 2019

This a proposal for building a library that will allow services to react to a DOM node's lifecycle, synchronously.

## Invariants
1. A node can be connected and disconnected from the DOM more than once. Every time the event happens, the qualifying hook should be invoked.
2. For a given node, more than one service can be registered to react to the same lifecycle event. In which case, each hook should be invoked in the order of registration(FIFO).
2.1 Duplicate hooks for the same event type, for the same node will be ignored. For example, if a service registers the same connectedCallback more than once for the same node, the callback is invoked only once.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a particular reason for this? seems to slow down things... while event listeners and other similar mechanisms do not apply this filtering.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate event listeners on the same node do get filtered out. Here's an example to demonstrate: https://jsbin.com/weyewul/edit?html,js,console,output

#### Callback
- Option 1
```js
type ReactionCallback = (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this better

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and it is faster.

- RFC PR: (leave this empty)
- Lightning Web Component Issue: (leave this empty)

# Summary
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make all the headers level 1 to headers level 2 except for the title.

Suggested change
# Summary
## Summary


# Basic example

*Proposal 1*: One simple API that accepts hooks for various lifecycle events
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For completeness purposes, we should also add a way to remove the lifecycle hook from the element.

```

# Motivation
The browser provides ways to react to lifecycle events in the window or document. For example, [`DOMContentLoaded`](https://developer.mozilla.org/en-US/docs/Web/API/Window/DOMContentLoaded_event) event signals that the initial HTML document has been loaded and parsed. The [`load`](https://developer.mozilla.org/en-US/docs/Web/API/Window/load_event) event signals that the page is full loaded. Similarly, [`beforeunload`](https://developer.mozilla.org/en-US/docs/Web/API/WindowEventHandlers/onbeforeunload) allows reactions to the resources being unloaded.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO making a parallel with MutationObserver would make more sense here.

This section should also highlight why MO doesn't work in order to emulate custom elements since the MO callback is invoked in a microtask while the CEReaction is invoked in place.


Web Components supports reacting to the lifecycle of a custom element. Hooks for lifecycle events are part of the custom component definition. Read the spec [here](https://developer.mozilla.org/en-US/docs/Web/Web_Components/Using_custom_elements#Using_the_lifecycle_callbacks) for more detail.

Lightning Web Components(LWC) has support for similar lifecycle events. Of particular interest to this RFC is the `connectedCallback` and `disconnectedCallback`. In LWC, the responsibility of invoking the hooks is distributed between the engine itself and snabdom. The goal of this RFC is to abstract the responsibility of managing the lifecycle into a library and make LWC subscribe to lifecycle events from the node-reactions library.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if we want to call out snabdom in this paragraph. I am envisioning completely/partially removing snabdom in the future.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah... agree, but lets make sure that we mention that today, engine's code is responsible for the hooks, and we don't like that... it is a problem to be solved.

# Motivation
The browser provides ways to react to lifecycle events in the window or document. For example, [`DOMContentLoaded`](https://developer.mozilla.org/en-US/docs/Web/API/Window/DOMContentLoaded_event) event signals that the initial HTML document has been loaded and parsed. The [`load`](https://developer.mozilla.org/en-US/docs/Web/API/Window/load_event) event signals that the page is full loaded. Similarly, [`beforeunload`](https://developer.mozilla.org/en-US/docs/Web/API/WindowEventHandlers/onbeforeunload) allows reactions to the resources being unloaded.

Web Components supports reacting to the lifecycle of a custom element. Hooks for lifecycle events are part of the custom component definition. Read the spec [here](https://developer.mozilla.org/en-US/docs/Web/Web_Components/Using_custom_elements#Using_the_lifecycle_callbacks) for more detail.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Web Components supports reacting to the lifecycle of a custom element. Hooks for lifecycle events are part of the custom component definition. Read the spec [here](https://developer.mozilla.org/en-US/docs/Web/Web_Components/Using_custom_elements#Using_the_lifecycle_callbacks) for more detail.
Web Components supports reacting to the lifecycle of a custom element. Hooks for lifecycle events are part of the custom component definition. Read the spec [here](https://developer.mozilla.org/en-US/docs/Web/Web_Components/Using_custom_elements#Using_the_lifecycle_callbacks) for more detail.

* @param {String} reactionEventType - one of connected, disconnected, adopted etc
* @param {Function} callback
*/
function reactTo(node, reactionEventType, callback);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally like better the approach with a single method used to that can be used for multiple type of node reaction (à la EventTarget.addEventListener(type, cb)).


# Alternatives

## MutationObserver
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, this section should be in moved to the motivation section.


# Unresolved questions

- How will traversal work for native custom elements running in closed shadow mode? In web components, Lifecycle hooks behave the same regardless of mode of shadow DOM. For example: https://jsbin.com/misofod/edit?html,js,console,output
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will we ever need to take care of this case?


Why should we *not* do this? Please consider:

- Performance: Traversing the dom is not cheap. We need to think about memoization techniques to prevent repetitive traversal. This is of significant importance since the invocations are synchronous in nature.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before merging this RFC I would be interested to measure the performance impact of such library on an existing application. Depending on the results we sill be able to better understand the trade-offs here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is important to notice that today, LWC is patching the same APIs, this is just a more complete package.

ChildNode.replaceWith

Range.insertNode

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change


# Adoption strategy

The usage of this library in LWC will be an implementation detail of the engine. Component developers should notive no difference in usage.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The usage of this library in LWC will be an implementation detail of the engine. Component developers should notive no difference in usage.
The usage of this library in LWC will be an implementation detail of the engine. Component developers should notice no difference in usage.

2. isConnected works for shadowRoot node and other documentFragments
3. When a node is moved to a different subtree, invoke its disconnectedCallback first and then the connectedCallback: https://jsbin.com/qawacil/edit?html,js,output
4. All hooks for a given node is processed first before moving down the subtree: https://jsbin.com/qawacil/edit?html,js,output
5. Hooks in subtree are invoked even in closed shadow mode.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. When a documentFragment is appended to a node, only the children of the documentFragment are connected to the document. So should the connectedCallback of the documentFragment be invoked?

3. Hooks are invoked synchronously.
4. In case of an event affecting a subtree, nodes are processed in [preorder(or treeorder)](https://dom.spec.whatwg.org/#concept-tree-order).
### Invariants
1. A DOM element can be connected and disconnected from the DOM more than once. Every time the event happens, the qualifying hook should be invoked.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hook -> callback

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are still few places where you call hook instead of callback, can we get them all to be the same?

ravijayaramappa added a commit to salesforce/lwc that referenced this pull request Sep 11, 2019
Library to react to dom elements connecting and disconnecting from the document
RFC: salesforce/lwc-rfcs#11
ravijayaramappa added a commit to salesforce/lwc that referenced this pull request Oct 2, 2019
Library to react to dom elements connecting and disconnecting from the document
RFC: salesforce/lwc-rfcs#11
@@ -0,0 +1,200 @@
---
title: Server Side Rendering
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you mean Node reactions?

status: DRAFTED
created_at: 2019-02-12
updated_at: 2019-05-24
pr: https://github.com/salesforce/lwc-rfcs/pull/2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#11 ?

text/0000-node-reactions.md Outdated Show resolved Hide resolved
text/0000-node-reactions.md Outdated Show resolved Hide resolved
ravijayaramappa added a commit to salesforce/lwc that referenced this pull request Feb 29, 2020
Library to react to dom elements connecting and disconnecting from the document
RFC: salesforce/lwc-rfcs#11
ravijayaramappa added a commit to salesforce/lwc that referenced this pull request Apr 2, 2020
Library to react to dom elements connecting and disconnecting from the document
RFC: salesforce/lwc-rfcs#11
ravijayaramappa added a commit to salesforce/lwc that referenced this pull request Apr 4, 2020
Library to react to dom elements connecting and disconnecting from the document
RFC: salesforce/lwc-rfcs#11
@pmdartus pmdartus added the draft label Oct 26, 2021
@ravijayaramappa ravijayaramappa deleted the ravi/master/node-reactions branch August 9, 2022 17:43
@ravijayaramappa
Copy link
Contributor Author

We have abandoned this proposal mainly due to its perf impact. There are alternates being considered like using the native custom element registry for LWCs and use the native connectedCallback and disconnectedCallback.

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

Successfully merging this pull request may close these issues.

4 participants