-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
fix(customElement): preserve appContext during hmr reload #12455
base: main
Are you sure you want to change the base?
Conversation
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-sfc
@vue/compiler-dom
@vue/compiler-ssr
@vue/runtime-core
@vue/runtime-dom
@vue/reactivity
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
@@ -520,7 +520,9 @@ export class VueElement | |||
} | |||
|
|||
private _update() { | |||
render(this._createVNode(), this._root) | |||
const vnode = this._createVNode() | |||
if (this._app && !this._instance) vnode.appContext = this._app._context |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason for checking this._app
here is that the test case will not pass.
core/packages/runtime-dom/__tests__/customElement.spec.ts
Lines 137 to 147 in b7b0932
test('remove then insert again', async () => { | |
container.innerHTML = `<my-element></my-element>` | |
const e = container.childNodes[0] as VueElement | |
container.removeChild(e) | |
await nextTick() | |
expect(e._instance).toBe(null) | |
expect(e.shadowRoot!.innerHTML).toBe('') | |
container.appendChild(e) | |
expect(e._instance).toBeTruthy() | |
expect(e.shadowRoot!.innerHTML).toBe('<div>hello</div>') | |
}) |
When re-inserting,
this._app
is null
because we cleared it in disconnectedCallback
.For custom elements that remove and then re-insert (
this._resolved = true
), just calling _update
is not enough. core/packages/runtime-dom/src/apiCustomElement.ts
Lines 300 to 303 in 2235643
if (this._resolved) { | |
this._setParent() | |
this._update() | |
} else { |
We must also reinitialize
this._ob
because it is cleared in disconnectedCallback
.For more edge cases refer to #12412. Maybe once #12413 is merged, the check for
this._app
can be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no suggestions.
Comments skipped due to low confidence (1)
packages/runtime-dom/src/apiCustomElement.ts:524
- The new behavior introduced in the '_update' method should be covered by tests to ensure that 'vnode.appContext' is correctly set.
if (this._app && !this._instance) vnode.appContext = this._app._context
close #12453