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

chore(SettingsAPI): replace reactive with ref to remove Vue 2 warn #13266

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Sep 11, 2024

Changes nothing in how it works.

Gets rid of the annoying Vue 2 warning about reactive(array) behavior on Vue 2 vs Vue 3 in watch (which we even don't use).

image

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy Antreesy force-pushed the chore/shallow-reactive-array-warn branch from 0b58e7a to b991291 Compare September 11, 2024 12:09
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Tested

  • adjusted test and add TODO comment

@Antreesy Antreesy merged commit 229f3b0 into main Sep 11, 2024
46 checks passed
@Antreesy Antreesy deleted the chore/shallow-reactive-array-warn branch September 11, 2024 12:20
@Antreesy
Copy link
Contributor

/backport to stable30

Comment on lines +26 to +27
// TODO: use shallowReactive instead of ref + markRaw in Vue 3 (see file commit history)
const customSettingsSections: Ref<TalkSettingsSection[]> = ref([])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

shallowReactive has no real advantages except that it allows to get rid of markRaw in just one function.

Copy link
Contributor

Choose a reason for hiding this comment

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

which still isn't bad, i think

This pull request was closed.
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.

3 participants