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

Models sections: non-mutating items computed #6480

Conversation

distantnative
Copy link
Member

Description

Summary of changes

  • Pages and files section: make that items computed maps the items by returning new objects instead of mutating the model parameter directly

Reasoning

In Vue 3 the page/file parameter inside the map closure will be reactive. So assigning new values will directly mutate the original object. Which is problematic in this case https://github.com/getkirby/kirby/compare/v5/develop...v5/refactor/nonmutating-modelssection-items-computed?expand=1#diff-97e5d0547fc36db8267a17f7b8e71ce28763c11dcba313d445cef261fce2ebc0R25 where we add the status button to other buttons. When the object is reactive, this adds an additional status button to the list, each time the computed prop code runs. By refactoring the code this is avoided.

Ready?

  • Tests and checks all pass

@distantnative distantnative added the type: refactoring ♻️ Is about bad code; cleans up code label Jun 9, 2024
@distantnative distantnative self-assigned this Jun 9, 2024
@distantnative distantnative added this to the 5.0.0-alpha.1 milestone Jun 9, 2024
@distantnative distantnative marked this pull request as ready for review June 9, 2024 09:22
@distantnative distantnative requested a review from a team June 9, 2024 09:22
@bastianallgeier bastianallgeier merged commit 932cf27 into v5/develop Jun 12, 2024
4 checks passed
@bastianallgeier bastianallgeier deleted the v5/refactor/nonmutating-modelssection-items-computed branch June 12, 2024 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: refactoring ♻️ Is about bad code; cleans up code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants