Replace 'forEach' with 'map' if mutating original data is not the goal
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 214
- Forks
- 52
- Avg merge
- 1d 10h
- Merged PRs (30d)
- 82
Description
forEach would change the column.selectionOptions objects directly (since filter doesn’t create copies). Was that the intention? If the goal is only to adjust the labels for display, maybe map could be a safer choice to avoid mutating the original data?
Originally posted by @silverkszlo in https://github.com/nextcloud/tables/pull/1975#discussion_r2282837014
Yeah, that's a good point. I copied this function from the existing code at https://github.com/nextcloud/tables/blob/038a943b443e8ba6e99074b8ed721d4f03ee2b09/src/shared/components/ncTable/partials/rowTypePartials/SelectionMultiForm.vue#L48. Unrelated to inline editing so should be updated separately everywhere.
Originally posted by @enjeck in https://github.com/nextcloud/tables/pull/1975#discussion_r2282847774
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with src/shared/components/ncTable/partials/rowTypePartials/SelectionMultiForm.vue at the referenced selection-options function, then locate the other copied uses mentioned in the issue. Check whether labels are only being adjusted for display and update the relevant uses so the original selection data is not mutated; done means the affected uses are consistent and existing tests still pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- frontend
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100