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

perf: improve scoped slots change detection accuracy #9371

Merged
merged 1 commit into from Jan 26, 2019

Conversation

Projects
3 participants
@yyx990803
Copy link
Member

yyx990803 commented Jan 25, 2019

Ensure that state mutations that only affect parent scope only trigger parent update and does not affect child components with only scoped slots.

Implications:

  • For optimal performance, scoped slots should always be preferred. (In v3 all slots are unified to work like scoped slots so that will be the default)

  • In 2.6, any slots using the new v-slot syntax are compiled as scopedSlots internally. This doesn't affect the child if it's using template, since <slot/> can render both scoped and normal slots, but if the child is using render functions, it is recommended to always use this.$scopedSlots. It is safe to do so in 2.6 since all normal slots are proxied on this.$scopedSlots as well.

  • Unfortunately, any child components with static slot content still need to be forced updated. This means the common use case of <parent><child></child></parent> doesn't benefit from this change, unless the default slot is explicitly forced into a scoped slot by using <parent v-slot:default><child></child></parent>. (We cannot directly force all slots into scoped slots as that would break existing render function code that expects the slot to be present on this.$slots instead of `this.$scopedSlots)

@yyx990803 yyx990803 added this to In progress in 2.6 Jan 25, 2019

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 25, 2019

Codecov Report

Merging #9371 into dev will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #9371      +/-   ##
==========================================
+ Coverage   97.33%   97.34%   +<.01%     
==========================================
  Files         107      107              
  Lines        4544     4551       +7     
==========================================
+ Hits         4423     4430       +7     
  Misses        121      121
Impacted Files Coverage Δ
src/core/vdom/helpers/normalize-scoped-slots.js 94.44% <100%> (+0.32%) ⬆️
src/compiler/codegen/index.js 100% <100%> (ø) ⬆️
src/compiler/parser/index.js 98.88% <100%> (+0.01%) ⬆️
src/core/instance/render-helpers/resolve-slots.js 100% <100%> (ø) ⬆️
src/core/instance/lifecycle.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 770c6ed...d91a38a. Read the comment docs.

@yyx990803 yyx990803 merged commit f219bed into dev Jan 26, 2019

8 checks passed

ci/circleci: install Your tests passed on CircleCI!
Details
ci/circleci: lint-flow-types Your tests passed on CircleCI!
Details
ci/circleci: test-cover Your tests passed on CircleCI!
Details
ci/circleci: test-e2e Your tests passed on CircleCI!
Details
ci/circleci: test-ssr-weex Your tests passed on CircleCI!
Details
ci/circleci: trigger-regression-test Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 97.33%)
Details
codecov/project 97.34% (+<.01%) compared to 770c6ed
Details

2.6 automation moved this from In progress to Done Jan 26, 2019

@johnsoncheg

This comment has been minimized.

Copy link

johnsoncheg commented Feb 17, 2019

Did the last sentence of the implications said that it's not recommended to use the new syntax to force transfer the normal slot to scoped slot according to the phase We cannot directly force all slots into scoped slots as that would break existing render function code that expects the slot to be present on this.$slotsinstead ofthis.$scopedSlots` @yyx990803

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment