Skip to content

Commit

Permalink
Codemod Object.assign with object literal first argument to object …
Browse files Browse the repository at this point in the history
…spread in Xplat

Summary:
Codemod `Object.assign` with object literal first argument (e.g. `Object.assign({}, foo)`) to object spread.

This adds several suppressions for exposed errors. The codemod produces errors as `Object.assign` is more unsafe than object spread. For example, `Object.assign` doesn't handle indexers, nor does it handle inexact objects properly, and when the (currently unsealed) empty object is supplied as the first argument, it also leads to unsafe behaviour:
https://flow.org/try/#0FAehAIGJwSQOwCYFMAeSBOBnYyDGAbAQ3SXADdjwBbQgBwC5wBvAbUwBd0BLOAcwF1GHbnwC+AbmDAAFAHkARgCskudgDpCmTF15xpTQowCMogDTU6ASkbyA9rfxJCcS+PBhwAfm8ymwcAGG4Eam-gFqETS0oaKu7hAAyrQkhAjgGOi2WOCa6Si0KuxICFIe0PCohKo5AGZF6HlV7DgqRCTklDyVqowGQpw8vOYRahJSCsqqGlo6en3gAEQAFlwL5vLGZuBdKE1xHjtN4Li2AK74abZkGVzI4AAG8vfgUvphOYzLq6EB4BvBP3CEUOqhi+0SyScaQyWUwOThqAKqmKpQg0AAqnBME5HGkalwsOwcuheDIJoVptpdPotvMTNZmDV7Iw4KcqPIMLE3B5vN4gA

The codemod is safe to do, despite some Flow errors, as `Object.assign` and object spread are equivalent at runtime, with the exception that `Object.assign` triggers setters on the target object. However the codemod [does not run](https://github.com/eslint/eslint/blob/938dbdd6c310784cc8a7329efaeb0e34321b9e1f/lib/rules/prefer-object-spread.js#L283-L285) if the first argument (object literal) has getters/setters, so we are fine.

```
ag -l 'Object.assign\(' | xargs ag -l 'flow' | xargs js1 lint --rule '{"prefer-object-spread":2}' --fix
```
Some manual fixes
```
arc f
```

Reviewed By: SamChou19815

Differential Revision: D36023786

fbshipit-source-id: b682562e670410acf4175ba59ab285c7bdcfe052
  • Loading branch information
gkz authored and facebook-github-bot committed Apr 29, 2022
1 parent 07a63a2 commit d992ae0
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 45 deletions.
17 changes: 10 additions & 7 deletions Libraries/Components/StatusBar/StatusBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,17 @@ function mergePropsStack(
propsStack: Array<Object>,
defaultValues: Object,
): Object {
return propsStack.reduce((prev, cur) => {
for (const prop in cur) {
if (cur[prop] != null) {
prev[prop] = cur[prop];
return propsStack.reduce(
(prev, cur) => {
for (const prop in cur) {
if (cur[prop] != null) {
prev[prop] = cur[prop];
}
}
}
return prev;
}, Object.assign({}, defaultValues));
return prev;
},
{...defaultValues},
);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion Libraries/Inspector/ElementBox.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ type Style = {
* @return a modified copy
*/
function resolveRelativeSizes(style: $ReadOnly<Style>): Style {
let resolvedStyle = Object.assign({}, style);
let resolvedStyle = {...style};
resolveSizeInPlace(resolvedStyle, 'top', 'height');
resolveSizeInPlace(resolvedStyle, 'right', 'width');
resolveSizeInPlace(resolvedStyle, 'bottom', 'height');
Expand Down
62 changes: 26 additions & 36 deletions Libraries/LogBox/Data/__tests__/LogBoxData-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,24 +70,19 @@ const addLogs = (logs, options) => {

const addSoftErrors = (errors, options) => {
errors.forEach(error => {
LogBoxData.addException(
Object.assign(
{},
{
message: '',
isComponentError: false,
originalMessage: '',
name: 'console.error',
componentStack: '',
stack: [],
id: 0,
isFatal: false,
},
typeof error === 'string'
? {message: error, originalMessage: error}
: error,
),
);
LogBoxData.addException({
message: '',
isComponentError: false,
originalMessage: '',
name: 'console.error',
componentStack: '',
stack: [],
id: 0,
isFatal: false,
...(typeof error === 'string'
? {message: error, originalMessage: error}
: error),
});
if (options == null || options.flush !== false) {
jest.runOnlyPendingTimers();
}
Expand All @@ -96,24 +91,19 @@ const addSoftErrors = (errors, options) => {

const addFatalErrors = (errors, options) => {
errors.forEach(error => {
LogBoxData.addException(
Object.assign(
{},
{
message: '',
isComponentError: false,
originalMessage: '',
name: 'console.error',
componentStack: '',
stack: [],
id: 0,
isFatal: true,
},
typeof error === 'string'
? {message: error, originalMessage: error}
: error,
),
);
LogBoxData.addException({
message: '',
isComponentError: false,
originalMessage: '',
name: 'console.error',
componentStack: '',
stack: [],
id: 0,
isFatal: true,
...(typeof error === 'string'
? {message: error, originalMessage: error}
: error),
});
if (options == null || options.flush !== false) {
// Errors include two timers, the second is for optimistic symbolication.
jest.runOnlyPendingTimers();
Expand Down
2 changes: 1 addition & 1 deletion Libraries/Utilities/truncate.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const truncate = function (
maxChars: number,
options?: truncateOptions,
): ?string {
options = Object.assign({}, defaultOptions, options);
options = {...defaultOptions, ...options};
if (
str &&
str.length &&
Expand Down

0 comments on commit d992ae0

Please sign in to comment.