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(devtools): use cleanupBuffer instead of modifying _buffer #6812

Merged
merged 3 commits into from
Oct 3, 2022
Merged

fix(devtools): use cleanupBuffer instead of modifying _buffer #6812

merged 3 commits into from
Oct 3, 2022

Conversation

posva
Copy link
Member

@posva posva commented Oct 3, 2022

At the moment the Playground SFC is broken when devtools are installed. I noticed _buffer is not always present so I adapted the type but maybe this isn't the right fix
cc @Akryum

@posva posva added ❗ p4-important Priority 4: this fixes bugs that violate documented behavior, or significantly improves perf. scope: playground labels Oct 3, 2022
@Akryum
Copy link
Member

Akryum commented Oct 3, 2022

In iframes the hook object is different: https://github.com/vuejs/devtools/blob/8dd0ef228a6c6a3913082b0fc2f34bffb777f6fc/packages/app-backend-core/src/hook.ts#L78-L105

@posva @antfu
Update packages/runtime-core/src/devtools.ts
fb80ebe 
Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>
@Akryum
Copy link
Member

Akryum commented Oct 3, 2022

vuejs/devtools@f2ad51e

@Akryum Akryum mentioned this pull request Oct 3, 2022
Closed
@Akryum Akryum changed the title fix(devtools): check _buffer existance fix(devtools): use cleanupBuffer instead of modifying _buffer Oct 3, 2022
@Akryum Akryum merged commit 35a113e into main Oct 3, 2022
@Akryum Akryum deleted the fix/devtools branch October 3, 2022 09:29
@segevfiner
Copy link

@posva @Akryum We hit a serious performance issue in dev mode in Vue v3.2.41 vs v3.2.40 which forced us to pin our version and we suspect it was caused by this PR as the performance tools point to cleanupBuffer. This is on a page which mounts/unmounts components rather rapidly, when I have the time I'll try to create a reproduction and report separately.

@matej-svejda
Copy link

matej-svejda commented Jan 10, 2023 •
edited
Loading

@posva @Akryum We hit a serious performance issue in dev mode in Vue v3.2.41 vs v3.2.40 which forced us to pin our version and we suspect it was caused by this PR as the performance tools point to cleanupBuffer. This is on a page which mounts/unmounts components rather rapidly, when I have the time I'll try to create a reproduction and report separately.

We ran into the same issue. Unmounting of components in dev mode when devtools is installed is significantly slower (if I would have to guess by a factor of 10). Uninstalling devtools resolves that.

@Akryum
Copy link
Member

Akryum commented Jan 10, 2023 •
edited
Loading

This performance issue should hopefully be fixed by vuejs/devtools@60aeed2

chrislone pushed a commit to chrislone/core that referenced this pull request Feb 4, 2023
@posva @antfu
fix(devtools): use cleanupBuffer instead of modifying _buffer (vuejs#…
01994cd 
…6812)

Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antfu antfu antfu approved these changes

@Akryum Akryum Akryum approved these changes

Assignees
No one assigned
Labels
❗ p4-important Priority 4: this fixes bugs that violate documented behavior, or significantly improves perf. scope: playground
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@posva @Akryum @segevfiner @matej-svejda @antfu