forked from facebook/hermes
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix unnecessary newlines being added during printing
Summary: There are cases where prettier inserts spaces in between detached nodes. Mainly this impacted function arguments as we'd do things like: ``` [].map(foo(a, b) {}) // after replacing with an arrow became: [].map( ( a, b, ) => {}, ) ``` This is bad for obvious reasons - because it is purely junk lines. From extensive testing, I found that the issue ONLY occurs if there is a docblock. My hypothesis is that when prettier lays out things like function parameters it looks to see if there is a comment on the line. If there is then it puts the param on the next line. Though - I don't have a true root cause for this as I haven't properly dug into prettier's source code and it's a pain to do remote nodejs debugging from a devserver and I cannot be bothered checking out locally. I tried a few fixes, but from my testing this fix (setting detached nodes to have range `[1, 1]`) works for cases of brand new detached nodes, and cloned detached nodes. Reviewed By: pieterv Differential Revision: D37499940 fbshipit-source-id: 6398186696692c047a6e4cb65a3901d57ef8745b
- Loading branch information
1 parent
dd34c0b
commit 49366a9
Showing
11 changed files
with
633 additions
and
18 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
118 changes: 118 additions & 0 deletions
118
...rser/js/hermes-transform/__tests__/test_codemods/removeUnnecessaryArrayMethodBind-test.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,118 @@ | ||
/** | ||
* Copyright (c) Meta Platforms, Inc. and affiliates. | ||
* | ||
* This source code is licensed under the MIT license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
* | ||
* @flow strict-local | ||
* @format | ||
*/ | ||
|
||
import {t, transform} from './test-utils'; | ||
|
||
const ARRAY_ITERATOR_METHODS = new Set([ | ||
'every', | ||
'filter', | ||
'forEach', | ||
'map', | ||
'some', | ||
// reduce / reduceRight don't accept a context argument | ||
]); | ||
|
||
function codemod(code: string) { | ||
return transform(code, context => { | ||
return { | ||
CallExpression(node) { | ||
if ( | ||
node.callee.type !== 'MemberExpression' || | ||
node.callee.computed === true || | ||
node.callee.property.type !== 'Identifier' || | ||
!ARRAY_ITERATOR_METHODS.has(node.callee.property.name) || | ||
node.arguments.length !== 2 | ||
) { | ||
return; | ||
} | ||
const callback = node.arguments[0]; | ||
const thisContext = node.arguments[1]; | ||
|
||
switch (callback.type) { | ||
case 'ArrowFunctionExpression': | ||
// arr.forEach(() => {}, this); | ||
// ^^^^ remove | ||
context.replaceNode( | ||
node, | ||
context.shallowCloneNodeWithOverrides(node, { | ||
arguments: [context.shallowCloneNode(node.arguments[0])], | ||
}), | ||
); | ||
break; | ||
|
||
case 'FunctionExpression': | ||
if (callback.generator) { | ||
// this isn't possible to directly convert to an arrow function | ||
// the second arg binding is really the only solution | ||
// but really nobody should ever be using an array iterator with a | ||
// generator function - that's just weird. | ||
return; | ||
} | ||
if (thisContext.type !== 'ThisExpression') { | ||
// nobody should be doing anything dumb like this to bind to a weird | ||
// this context, but it is valid code that can't be done with an | ||
// arrow function | ||
return; | ||
} | ||
// arr.forEach(function foo() {}, this); | ||
// ^^^^ remove | ||
// ^^^^^^^^^^^^^^^^^ convert to arrow | ||
context.replaceNode( | ||
node, | ||
context.shallowCloneNodeWithOverrides(node, { | ||
arguments: [ | ||
t.ArrowFunctionExpression({ | ||
async: callback.async, | ||
body: context.shallowCloneNode(callback.body), | ||
params: context.shallowCloneArray(callback.params), | ||
predicate: context.shallowCloneNode(callback.predicate), | ||
returnType: context.shallowCloneNode(callback.returnType), | ||
typeParameters: context.shallowCloneNode( | ||
callback.typeParameters, | ||
), | ||
}), | ||
], | ||
}), | ||
); | ||
} | ||
}, | ||
}; | ||
}); | ||
} | ||
|
||
describe('remove unnecessary array method bind', () => { | ||
it('should work', () => { | ||
const result = codemod(`\ | ||
/** | ||
*/ | ||
export default class Component { | ||
method() { | ||
return LINKS.map(function (d, idx) { | ||
return 1; | ||
}, this); | ||
} | ||
} | ||
`); | ||
|
||
expect(result).toBe(`\ | ||
/** | ||
*/ | ||
export default class Component { | ||
method() { | ||
return LINKS.map((d, idx) => { | ||
return 1; | ||
}); | ||
} | ||
} | ||
`); | ||
}); | ||
}); |
Oops, something went wrong.