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.

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(keep-alive): The value of include is empty string should cache no… #11369

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dddssw
Copy link

@dddssw dddssw commented Jul 16, 2024

…thing(#11366)

Copy link
Member

@yyx990803 yyx990803 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a test case for this?

@@ -220,7 +220,8 @@ const KeepAliveImpl: ComponentOptions = {
watch(
() => [props.include, props.exclude],
([include, exclude]) => {
include && pruneCache(name => matches(include, name))
;(include || typeof include === 'string') &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can just be include != null

@@ -220,7 +220,8 @@ const KeepAliveImpl: ComponentOptions = {
watch(
() => [props.include, props.exclude],
([include, exclude]) => {
include && pruneCache(name => matches(include, name))
;(include || typeof include === 'string') &&
pruneCache(name => matches(include, name))
exclude && pruneCache(name => !matches(exclude, name))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency, also check for exclude != null

@@ -300,7 +301,8 @@ const KeepAliveImpl: ComponentOptions = {
const { include, exclude, max } = props

if (
(include && (!name || !matches(include, name))) ||
((include || typeof include === 'string') &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

@dddssw
Copy link
Author

dddssw commented Jul 18, 2024

This is my first time writing test cases, and I'm not sure if I should write it this way.thank

@edison1105
Copy link
Member

/ecosystem-ci run

@vue-bot
Copy link

vue-bot commented Jul 19, 2024

📝 Ran ecosystem CI: Open

suite result latest scheduled
language-tools failure failure
nuxt success success
pinia success success
primevue success success
quasar success success
radix-vue success success
router success success
test-utils success success
vant success success
vite-plugin-vue success success
vitepress success success
vue-i18n failure success
vue-macros success success
vuetify success success
vueuse success success
vue-simple-compiler success success

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

Successfully merging this pull request may close these issues.

None yet

4 participants