Infinite Loop Attaching Behavior #4
Comments
|
Hey Jon, thanks for reporting, and interesting to see someone using this. I've actually made some refactors to this part of the code, about to release those today. Do you have a snippet of code I can use to reproduce it? I updated the codepen to pull in the latest version: |
|
I gave it a shot like the following, but it works: <html>
<head>
<script src="https://unpkg.com/[email protected]/dist/global.js"></script>
<script>
class ClickCounter {
constructor(element) {
this.element = element
this.count = 0
}
connectedCallback() {
this.render()
this.element.addEventListener('click', () => {
this.count++
this.render()
})
}
render() {
this.element.textContent = `count: ${this.count}`
}
}
elementBehaviors.define('click-counter', ClickCounter)
class ClickLogger {
constructor(element) {
this.element = element
}
connectedCallback() {
this.element.addEventListener('click', () => {
console.log('clicked on element: ', this.element)
})
}
}
elementBehaviors.define('click-logger', ClickLogger)
</script>
</head>
<body>
<span>
<input has="click-counter click-logger" />
</span>
</body>
</html> |
|
Yes, I like the idea of Here's an example of an infinite loop.
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Document</title>
<script src="https://unpkg.com/[email protected]/dist/global.js"></script>
</head>
<body>
<script src="./infinite-loop.js"></script>
<input has="infinite-loop" value="Infitely looping!">
</body>
</html>
class InfiniteLoop {
constructor(element) {
this.element = element
}
connectedCallback() {
let element = this.element
let span = document.createElement('span')
element.parentNode.insertBefore(span, element)
span.appendChild(element)
}
}
elementBehaviors.define('infinite-loop', InfiniteLoop)This results in the A different solution to this problem could be adding a |
|
Ah, thank you for the repro. This isn't something that can be solved in element-behaviors, or even in custom elements API. The code is just doing what you're telling it to: you disconnect and then re-connect the element (with Here is the same problem, but with custom elements: https://codepen.io/trusktr/pen/jOzGJJg?editors=1010 The only solution is for user code to not do this. As an example, here's one way user code could solve it: https://codepen.io/trusktr/pen/wvmrOZO?editors=1010 Fixing this in element-behaviors would mean that we have changed the contract of how As a side note, I would consider this sort of DOM patching to be an anti pattern: custom elements (or element behaviors) should not change the end user's perceived DOM. For example, if the user writes <some-element></some-element>that's what they should end with, otherwise if the DOM changes to something they didn't write, unexpectedly, as in <span><some-element></some-element></span>it means that As a rule of thumb, always prevent custom elements (or behaviors) from modifying outside DOM or attributes, because those outside things are the user's API contract (the user's input). There can be exceptions to the rule, but it is something to avoid as much as possible. |
No prob! I was pleasantly surprised someone used it. Thanks for the input! |
|
So, the way the built-in web components work is that the You can see how this is behaving with this: ;(function() {
class InfiniteLoop {
movingElement = false
createdCount
constructor(element) {
this.element = element
if (element._createdCount) {
this.createdCount = element._createdCount++
} else {
element._createdCount = 1
this.createdCount = 1
}
console.log(`Created count: ${this.createdCount}`)
}
connectedCallback() {
console.log(`Connected callback called for '${this.createdCount}'`)
let element = this.element
if (this.movingElement) return
let span = document.createElement('span')
element.parentNode.insertBefore(span, element)
this.movingElement = true
span.appendChild(element)
this.movingElement = false
}
}
elementBehaviors.define('infinite-loop', InfiniteLoop)
})();I think an elegant solution to this problem might be something like this (not tested): const associatedElements = new WeakMap
...
if (!Behavior)
return;
let behaviors
let existingBehavior
if ((behaviors = associatedElements.get(this.ownerElement)) && (existingBehavior = behaviors.find(x => x instanceof Behavior))) {
typeof existingBehavior.connectedCallback === "function" && existingBehavior.connectedCallback()
return
}
const behavior = new Behavior(this.ownerElement);
if (!behaviors) {
behaviors = []
associatedElements.set(this.ownerElement, behaviors)
}
behaviors.push(behavior)Using the |
|
Note, the way I'm debugging this is by commenting out the line |
Ah, good point! I was wondering if we should keep the behavior alive, so long as the element has the behavior listed in the
The upside of the monkey patching would be better behavior, more like we would see if this were a native API in the browser, but the downside is that polyfill code will get uglier and more hacky. In the meantime, here's how we can apply the same workaround as the custom element example, but with element behaviors: Infinite loop: https://codepen.io/trusktr/pen/WNzdGRB?editors=1000 Fixed: https://codepen.io/trusktr/pen/VwXyKMX?editors=1000 (the main trick is the I would still recommend to avoid this pattern. People in the community consider the act of affecting the element user's DOM to be an anti-pattern. Why do you need to patch the outside DOM? |
That's a nice attempt, but the only problem (that already exists regardless of your new code or the old code) is that the change mechanism does not work if the element is not connected. F.e., if we were to keep the behavior instances alive, then the following would happen: const el = document.querySelector('.some-el')
el.remove() // change detection for the has attribute stops working at this point.
el.setAttribute('has', '') // try to remove behaviors, but nothing happens
await new Promise(r => setTimeout(r, 1000)) // sleep one second
!!el.behaviors.get('some-behavior') // still `true`, which is awkward considering `has` attribute is empty
// only appending the el back into the DOM will cause the behaviors to update:
document.body.append(el)
!!el.behaviors.get('some-behavior') // still `true`, MutationObserver didn't fire yet.
await new Promise(r => setTimeout(r)) // wait for MutationObserver
!!el.behaviors.get('some-behavior') // finally `false`I think monkey patching would be the way to properly solve this (setAttribute, el.attributes, Attr, etc), and in fact will make the end result better in that behaviors could be modified (or their life cycle methods called) synchronously at the same time as custom elements would experience them, rather than always deferred in a microtask as with As a result of the monkey patching approach, that last solution example would not require a With monkey patching, that last code snippet would become this: const el = document.querySelector('.some-el')
el.remove()
el.setAttribute('has', '') // remove behaviors, it works
!!el.behaviors.get('some-behavior') // `false`, nice!
document.body.append(el)
!!el.behaviors.get('some-behavior') // still `false`, nice!and keeping the instances alive would then make great sense. Currently, by removing the instances immediately on disconnect, we're not left with any chance for the behaviors to exist when they shouldn't. I think this is better than the |
|
Mmm, yeah, I'm imagining monkey patching will be the way to go! The following is what we'll need to patch: To catch all construction/connected/disconnected cases:
To catch all attribute changes:
Did I miss any? |
|
Yeah, I'm not entirely sure about all the internals of what you are doing. So, whatever works! :-)
What I'm using this for is to change how the display works on a form <span has=input-currency><input type=number step="0.01"></span>But if I keep the solution decoupled from the HTML I have: <input type=number step="0.01" has=input-currency>So, if I want to change anything all I need to do is go into the Also, using the element behaviors library allows me to not have to rely on a JavaScript framework like React which decouples my code from complex frameworks in favor of a simple small classes with specific state management. Which makes it easier to implement a micro front end. So, that is my use case and rational :-). |
When connecting an
inputelement I wrap the tag in aspanand this causes theinputelement to be attached again in an infinite loop.A temporary fix to this that I did was to change the code around this line:
element-behaviors/src/index.ts
Line 183 in fb9f2aa
with the following code:
Maybe there is a better solution? I don't know. But it works for now.
It is interesting to note that the version used in the code pen example in the docs doesn't have this behavior. It only occurs in the latest version.
The text was updated successfully, but these errors were encountered: