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.

Sign up for GitHub

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

Jump to bottom

fix(runtime-core): TypeError when using html comment in teleport(fix #10747) #10748

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(runtime-core): TypeError when using html comment in teleport(fix #10747) #10748

wants to merge 1 commit into from

Conversation

tonliver
Copy link

close #10747

@tonliver tonliver changed the title fix #10747 fix(runtime-core): TypeError when using html comment in teleport[fix #10747] Apr 20, 2024
@netlify Netlify
Copy link

netlify bot commented Apr 20, 2024

❌ Deploy Preview for vue-sfc-playground failed.

Name Link
🔨 Latest commit 46f6f3b
🔍 Latest deploy log https://app.netlify.com/sites/vue-sfc-playground/deploys/66231674bd858b000859c462

@netlify Netlify
Copy link

netlify bot commented Apr 20, 2024

❌ Deploy Preview for vue-next-template-explorer failed.

Name Link
🔨 Latest commit 46f6f3b
🔍 Latest deploy log https://app.netlify.com/sites/vue-next-template-explorer/deploys/662316740aee4500083b5a0e

@tonliver tonliver changed the title fix(runtime-core): TypeError when using html comment in teleport[fix #10747] fix(runtime-core): TypeError when using html comment in teleport(fix #10747) Apr 20, 2024
@tonliver
Copy link
Author

@edison1105 I think assuming that all comments should be removed in production mode, then there shouldn't create vnodes for comments in production mode for performance reason. However, comment vnodes are created, also removed by nodeOpt.remove method during unmounting, except for traversStaticChildren, which lacks consistency. Indeed this inconsistency in handling caused the exception.

@edison1105
Copy link
Member

@tonliver Could you add a test case?

@edison1105 edison1105 added the need test The PR has missing test cases. label Apr 24, 2024
@tonliver
Copy link
Author

@tonliver Could you add a test case?

Sure, I'll work on it as soon as possible.

@edison1105 edison1105 mentioned this pull request May 6, 2024
Closed
@github-actions GitHub Actions
Copy link

github-actions bot commented May 6, 2024

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 90.8 kB (+60 B) 34.5 kB (+29 B) 31.1 kB (+31 B)
vue.global.prod.js 148 kB (+135 B) 53.8 kB (+64 B) 48 kB (+31 B)

Usages

Name Size Gzip Brotli
createApp 50.8 kB (+66 B) 19.9 kB (-7 B) 18.1 kB (+18 B)
createSSRApp 54.2 kB (+47 B) 21.2 kB (+4 B) 19.3 kB (+15 B)
defineCustomElement 53.1 kB (+66 B) 20.6 kB (+13 B) 18.8 kB (+21 B)
overall 64.6 kB (+82 B) 24.9 kB (+32 B) 22.6 kB (+42 B)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
need test The PR has missing test cases.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Throw TypeError when writing html comment in teleport
2 participants
@tonliver @edison1105