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 9f42f24 commit ac081ac
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 9 deletions.
2 changes: 1 addition & 1 deletion packages/metro-react-native-babel-transformer/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ function buildBabelConfig(
if (mayContainEditableReactComponents) {
const hmrConfig = makeHMRConfig();
hmrConfig.plugins = withExtrPlugins.concat(hmrConfig.plugins);
config = Object.assign({}, config, hmrConfig);
config = {...config, ...hmrConfig};
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/metro/src/Bundler/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ function filterObject(
object: AssetDataWithoutFiles,
blockList: Set<string>,
): AssetDataFiltered {
const copied = Object.assign({}, object);
const copied = {...object};
for (const key of blockList) {
delete copied[key];
}
Expand Down
12 changes: 5 additions & 7 deletions packages/metro/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,13 +259,11 @@ exports.runServer = async (
if (secure || secureServerOptions != null) {
let options = secureServerOptions;
if (typeof secureKey === 'string' && typeof secureCert === 'string') {
options = Object.assign(
{
key: fs.readFileSync(secureKey),
cert: fs.readFileSync(secureCert),
},
secureServerOptions,
);
options = {
key: fs.readFileSync(secureKey),
cert: fs.readFileSync(secureCert),
...secureServerOptions,
};
}
httpServer = https.createServer(options, serverApp);
} else {
Expand Down

0 comments on commit ac081ac

Please sign in to comment.