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

[NOQA] feat: disable eslint prefer object spread #49521

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ module.exports = {
// ESLint core rules
'es/no-nullish-coalescing-operators': 'off',
'es/no-optional-chaining': 'off',
'prefer-object-spread': 'off',

// Import specific rules
'import/consistent-type-specifier-style': ['error', 'prefer-top-level'],
Expand Down
22 changes: 22 additions & 0 deletions contributingGuides/STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
- [`var`, `const` and `let`](#var-const-and-let)
- [Object / Array Methods](#object--array-methods)
- [Accessing Object Properties and Default Values](#accessing-object-properties-and-default-values)
- [Object Merging: Spread Operator vs. Object.assign](#object-merging-spread-operator-vs-objectassign)
- [JSDocs](#jsdocs)
- [Component props](#component-props)
- [Destructuring](#destructuring)
Expand Down Expand Up @@ -737,6 +738,27 @@ const name = user?.name || "default name";
const name = user?.name ?? "default name";
```

## Object Merging: Spread Operator vs. Object.assign

Avoid using the spread operator `({...obj})` in performance-critical situations where object merging occurs frequently (e.g., in loops or repeated function calls). The spread operator creates unnecessary intermediate objects, which can lead to higher memory usage and slower execution.

Instead, consider using `Object.assign()` in these scenarios for better performance. The ESLint `prefer-object-spread` rule has been disabled to allow developers to make informed choices between readability and performance.

- Use the spread operator for simple, small object merges where performance isn't a concern.
- Use Object.assign() when working with large objects or in performance-sensitive contexts.

```ts
// BAD: Using the spread operator in a performance-critical loop
Copy link
Contributor

Choose a reason for hiding this comment

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

This example is not really clear about the performance aspect. Could we maybe add some context to the example (just on its own it does not seem the problematic). Maybe add it to some loop/ array_reduce

const errorSources = {
reportErrors,
...reportErrorFields,
...reportActionErrors,
};

// GOOD: Using Object.assign() for better performance in frequent merges
const errorSources = Object.assign({}, reportErrors, reportErrorFields, reportActionErrors);
```

## JSDocs

- Omit comments that are redundant with TypeScript. Do not declare types in `@param` or `@return` blocks. Do not write `@implements`, `@enum`, `@private`, `@override`. eslint: [`jsdoc/no-types`](https://github.com/gajus/eslint-plugin-jsdoc/blob/main/.README/rules/no-types.md)
Expand Down
1 change: 0 additions & 1 deletion src/libs/OptionsListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,6 @@ function getAllReportErrors(report: OnyxEntry<Report>, reportActions: OnyxEntry<
}
// All error objects related to the report. Each object in the sources contains error messages keyed by microtime
// Use Object.assign to merge all error objects into one since it is faster and uses less memory than spread operator
// eslint-disable-next-line prefer-object-spread
const errorSources = Object.assign({}, reportErrors, reportErrorFields, reportActionErrors);

// Combine all error messages keyed by microtime into one object
Expand Down
Loading