Fix single-tree-render codemod remove extra new lines only in modified files

[simison]

Removes extra new lines between two import statements only when file is modified.

Related to PR #19494 "Single tree-rendering-issues"

That removeExtraNewlines() turns this:

import { preloadReaderBundle, sidebar, updateLastRoute } from 'reader/controller'; 

import { makeLayout, render as clientRender } from 'controller';

into this:

import { preloadReaderBundle, sidebar, updateLastRoute } from 'reader/controller';
import { makeLayout, render as clientRender } from 'controller';

...since it's something Prettier doesn't do for us.

Previously in this codemod these kind of extra empty lines would be removed also from files this codemod didn't modify. Now they'll be removed only from files where we add that import makeLayout/clientRender.

Empty lines are caused by bug in insertAfter(): benjamn/recast#371

To test

npm run codemod single-tree-rendering ./client/my-sites/checkout/index.js

→ Adds new import, but no newlines

npm run codemod single-tree-rendering ./client/my-sites/domains/components/form/test/hidden-input.js

→ Doesn't add import and won't fix old empty lines between imports

[simison]

Ping @samouri & @jsnajdr do you think cleaning these empty lines could be part of calypso-prettier instead? Right now every codemod using insertAfter() adds these empty lines so this same fix has to be done in multiple places, while Prettier could just fix it for us. :-)

[simison]

@jsnajdr commented (#19555 (review)):

As a long term goal, it shouldn't be terribly hard to adapt jscodeshift to use Prettier to format the resulting AST instead of recast. That should solve a lot of problems, including the extra newlines in #19547.

While I totally agree with the sentiment, it might not solve this specific issue with empty lines between imports, see example (after opening the link, you might need to refresh the tab to see it properly).

Anyway, this will get us moving forward for now, but a proper fix for the issue is needed in long term.

[jsnajdr]

@simison I'd like to keep calypso-prettier as close to the original upstream version as possible. There is some maintenance burden that's already big enough. I think your fix with removeExtraNewLines is a better solution for now.

I'm experimenting with printing the codemodded AST with Prettier directly and avoiding the recast printer completely. Instead of:

return root.toSource( config.recastOptions );

which calls recast.print internally, let's call:

return prettier.__debug.formatAST( root.get().value, { originalText: file.source } ).formatted;

This doesn't solve the extra lines between import at the moment (Prettier keeps them there, too), but if we manage to make it work, it's a very promising approach.

[simison]

I'd like to keep calypso-prettier as close to the original upstream version as possible. There is some maintenance burden that's already big enough.

True, that's totally reasonable considering you have to rebase against new versions of Prettier each time.

I was thinking this another day: have you seen prettier/prettier-eslint? (They have also some other prettier/eslint* tools.)

My thinking is that rather than maintaining a prettier fork, we could maintain eslint plugin that works on top of it?

I'm experimenting with printing the codemodded AST with Prettier directly and avoiding the recast printer completely.

That's a great idea! I'll give this a try today.