-
Notifications
You must be signed in to change notification settings - Fork 92
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
Virtual Components #475
Comments
The virtual.test test file also runs with this solution |
For the |
My understanding is that the I am unable to reproduce the scenario you described. Could you please provide a test case for me? Thank you. |
I will provide a better example at a some later point but basically:
|
#475 (comment) Additionally, I am using [email protected] export const VirtualButton = virtual(() => {
useEffect(() => {
console.log('effect')
return () => {
console.log('effect-clear')
}
}, [])
return html`<button>button</button>`
})
const MyApp = () => {
const [count, setCount] = useState(0)
return html`<div>
<div>${count % 2 === 0 ? VirtualButton() : 'None'}</div>
</div>`
} output
|
Yes! this works. But does it work if you unmount/detach the my-app element ? from web inspector. |
Understood. If we only consider the unmounting of the |
If we handle this correctly by
in I don't know if we actually need to have a map of parts or schedulers . To me it looks like |
I've written some code, primarily to collect const virtualDirectives = new WeakMap<HTMLComponent<P>, Set<typeof Directive>>()
const setVirtualDirectives = (values: unknown[]): void => {
for (const value of values) {
if (isDirectiveResult(value)) {
const directive = getDirectiveClass(value)
if (!directive) continue
const directives = virtualDirectives.get(component) || new Set()
directives.add(directive)
virtualDirectives.set(component, directives)
// @ts-ignore
setVirtualDirectives(value.values ?? [])
}
}
} component scheduler render scheduler.render = () => {
const result = scheduler.state.run(() => renderer.call(component, component))
if (isTemplateResult(result)) {
setVirtualDirectives(result.values)
} else if (isDirectiveResult(result)) {
setVirtualDirectives([result])
} else if (Array.isArray(result)) {
setVirtualDirectives(result)
}
console.log(result,virtualDirectives)
return result
} component scheduler teardown const superTeardown = scheduler.teardown
scheduler.teardown = (): void => {
const directives = virtualDirectives.get(component)
if (directives) {
for (const directive of directives) {
// @ts-ignore
const instance = VIRTUAL_CLASS_TO_INSTANCE.get(directive)
if (instance) {
// @ts-ignore
instance['_$notifyDirectiveConnectionChanged'](false)
}
}
}
superTeardown()
} Now it meets the unmounting requirement for releasing the virtual component at the root node. However, in a structure like the one below, where the is removed using the DOM, it still doesn't release properly:
useEffect(() => {
// Removing <div> using the DOM
divRef.remove();
}, [])
return html`<div ${ref(divRef)}>${VirtualComponent()}</div>` I believe there should be a convention here, discouraging the direct use of the DOM to manipulate nodes rendered from HTML templates. What do you think? |
I am using this patch to detach on component disconnect: diff --git a/node_modules/haunted/lib/component.js b/node_modules/haunted/lib/component.js
index fae37bb..702cd4b 100644
--- a/node_modules/haunted/lib/component.js
+++ b/node_modules/haunted/lib/component.js
@@ -3,12 +3,13 @@ const toCamelCase = (val = '') => val.replace(/-+([a-z])?/g, (_, char) => char ?
function makeComponent(render) {
class Scheduler extends BaseScheduler {
frag;
+ renderResult;
constructor(renderer, frag, host) {
super(renderer, (host || frag));
this.frag = frag;
}
commit(result) {
- render(result, this.frag);
+ this.renderResult = render(result, this.frag);
}
}
function component(renderer, baseElementOrOptions, options) {
@@ -31,9 +32,11 @@ function makeComponent(render) {
}
connectedCallback() {
this._scheduler.update();
+ this._scheduler.renderResult?.setConnected(true)
}
disconnectedCallback() {
this._scheduler.teardown();
+ this._scheduler.renderResult?.setConnected(false)
}
attributeChangedCallback(name, oldValue, newValue) {
if (oldValue === newValue) {
|
Great! I'll take it with me. |
https://github.com/matthewp/haunted/blob/main/src/virtual.ts#L59
Why don't you use the disconnected method here?
Is there more performance consumption with MutationObserver?
The text was updated successfully, but these errors were encountered: