Skip to content

Commit eb3d3f8

Browse files
committed
[Refactor] Stop rerendering directives, inject imports instead
Summary: This commit changes how the sorted imports are combined with the original source. Prior to this commit, all ImportDeclaration nodes and their leading comments, plus any InterpreterDirective and Directive nodes, were extracted from the original code and re-rendered using babel. The rendered nodes were then concatenated with the original source with those nodes removed to produce the updated source. This approach safely protected against functional changes, but removed newlines around comments near the beginning of the file when the first node of the original source was an ImportDeclaration, as babel does not preserve whitespace when rendering content. If a user has configured a plugin that attempts to manage comments and/or whitespace near the top of the file, such as auto-inserting a license header (as I am trying to do), this results in conflicts / formatting churn. This commit does not directly resolve this incompatibility, however it better prepares the codebase for a plugin option to be added that can resolve the issue. Test Plan: `yarn install && yarn run test --all` Note that one snapshot was changed by this commit where a newline was changed, acting as an effective example of how the original approach could affect whitespace in the re-rendered portion of the file.
1 parent 042f1a0 commit eb3d3f8

File tree

8 files changed

+133
-121
lines changed

8 files changed

+133
-121
lines changed

src/preprocessors/preprocessor.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { ParserOptions, parse as babelParser } from '@babel/parser';
2-
import { Directive, ImportDeclaration } from '@babel/types';
2+
import { ImportDeclaration } from '@babel/types';
33

44
import { PrettierOptions } from '../types';
55
import { extractASTNodes } from '../utils/extract-ast-nodes';
@@ -26,12 +26,11 @@ export function preprocessor(code: string, options: PrettierOptions) {
2626
};
2727

2828
const ast = babelParser(code, parserOptions);
29-
const interpreter = ast.program.interpreter;
3029

3130
const {
3231
importNodes,
33-
directives,
34-
}: { importNodes: ImportDeclaration[]; directives: Directive[] } =
32+
injectIdx,
33+
}: { importNodes: ImportDeclaration[]; injectIdx: number } =
3534
extractASTNodes(ast);
3635

3736
// short-circuit if there are no import declaration
@@ -47,7 +46,7 @@ export function preprocessor(code: string, options: PrettierOptions) {
4746
importOrderSideEffects,
4847
});
4948

50-
return getCodeFromAst(allImports, directives, code, interpreter, {
49+
return getCodeFromAst(allImports, code, injectIdx, {
5150
importOrderImportAttributesKeyword,
5251
});
5352
}
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import { format } from 'prettier';
2+
3+
import { assembleUpdatedCode } from '../assemble-updated-code';
4+
import { getAllCommentsFromNodes } from '../get-all-comments-from-nodes';
5+
import { getImportNodes } from '../get-import-nodes';
6+
import { getSortedNodes } from '../get-sorted-nodes';
7+
8+
const code = `"use strict";
9+
// first comment
10+
// second comment
11+
import z from 'z';
12+
import c from 'c';
13+
import g from 'g';
14+
import t from 't';
15+
import k from 'k';
16+
// import a from 'a';
17+
// import a from 'a';
18+
import a from 'a';
19+
`;
20+
21+
test('it should remove nodes from the original code', async () => {
22+
const importNodes = getImportNodes(code);
23+
const sortedNodes = getSortedNodes(importNodes, {
24+
importOrder: [],
25+
importOrderCaseInsensitive: false,
26+
importOrderSeparation: false,
27+
importOrderGroupNamespaceSpecifiers: false,
28+
importOrderSortSpecifiers: false,
29+
importOrderSideEffects: true,
30+
});
31+
const allCommentsFromImports = getAllCommentsFromNodes(sortedNodes);
32+
33+
const commentAndImportsToRemoveFromCode = [
34+
...sortedNodes,
35+
...allCommentsFromImports,
36+
];
37+
const codeWithoutImportDeclarations = assembleUpdatedCode(
38+
code,
39+
commentAndImportsToRemoveFromCode,
40+
);
41+
const result = await format(codeWithoutImportDeclarations, {
42+
parser: 'babel',
43+
});
44+
expect(result).toEqual(`"use strict";
45+
`);
46+
});
47+
48+
test('it should inject the generated code at the correct location', async () => {
49+
const importNodes = getImportNodes(code);
50+
const sortedNodes = getSortedNodes(importNodes, {
51+
importOrder: [],
52+
importOrderCaseInsensitive: false,
53+
importOrderSeparation: false,
54+
importOrderGroupNamespaceSpecifiers: false,
55+
importOrderSortSpecifiers: false,
56+
importOrderSideEffects: true,
57+
});
58+
const allCommentsFromImports = getAllCommentsFromNodes(sortedNodes);
59+
60+
const commentAndImportsToRemoveFromCode = [
61+
...sortedNodes,
62+
...allCommentsFromImports,
63+
];
64+
const codeWithoutImportDeclarations = assembleUpdatedCode(
65+
code,
66+
commentAndImportsToRemoveFromCode,
67+
`import generated from "generated";`,
68+
'"use strict";'.length,
69+
);
70+
const result = await format(codeWithoutImportDeclarations, {
71+
parser: 'babel',
72+
});
73+
expect(result).toEqual(`"use strict";
74+
import generated from "generated";
75+
`);
76+
});
Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
1-
import { parse as babelParser } from '@babel/core';
2-
import { ParserOptions } from '@babel/parser';
31
import { format } from 'prettier';
42

5-
import { extractASTNodes } from '../extract-ast-nodes';
63
import { getCodeFromAst } from '../get-code-from-ast';
7-
import { getExperimentalParserPlugins } from '../get-experimental-parser-plugins';
84
import { getImportNodes } from '../get-import-nodes';
95
import { getSortedNodes } from '../get-sorted-nodes';
106

@@ -27,7 +23,7 @@ import a from 'a';
2723
importOrderSortSpecifiers: false,
2824
importOrderSideEffects: true,
2925
});
30-
const formatted = getCodeFromAst(sortedNodes, [], code, null);
26+
const formatted = getCodeFromAst(sortedNodes, code);
3127
expect(await format(formatted, { parser: 'babel' })).toEqual(
3228
`// first comment
3329
// second comment
@@ -40,29 +36,3 @@ import z from "z";
4036
`,
4137
);
4238
});
43-
44-
test('it renders directives correctly', async () => {
45-
const code = `
46-
"use client";
47-
// first comment
48-
import b from 'b';
49-
import a from 'a';`;
50-
51-
const parserOptions: ParserOptions = {
52-
sourceType: 'module',
53-
plugins: getExperimentalParserPlugins([]),
54-
};
55-
const ast = babelParser(code, parserOptions);
56-
if (!ast) throw new Error('ast is null');
57-
const { directives, importNodes } = extractASTNodes(ast);
58-
59-
const formatted = getCodeFromAst(importNodes, directives, code, null);
60-
expect(await format(formatted, { parser: 'babel' })).toEqual(
61-
`"use client";
62-
63-
// first comment
64-
import b from "b";
65-
import a from "a";
66-
`,
67-
);
68-
});

src/utils/__tests__/remove-nodes-from-original-code.spec.ts

Lines changed: 0 additions & 44 deletions
This file was deleted.

src/utils/remove-nodes-from-original-code.ts renamed to src/utils/assemble-updated-code.ts

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,32 +3,56 @@ import { Comment, Node } from '@babel/types';
33
type NodeOrComment = Node | Comment;
44
type BoundedNodeOrComment = NodeOrComment & { start: number; end: number };
55

6+
interface InjectedCode {
7+
type: 'InjectedCode';
8+
start: number;
9+
end: number;
10+
}
11+
612
/**
7-
* Removes imports from original file
13+
* Assembles the updated file, removing imports from the original file and
14+
* injecting the sorted imports at the appropriate location.
15+
*
816
* @param code the whole file as text
917
* @param nodes to be removed
18+
* @param injectedCode the generated import source to be injected
19+
* @param injectIdx the index at which to inject the generated source
1020
*/
11-
export const removeNodesFromOriginalCode = (
21+
export const assembleUpdatedCode = (
1222
code: string,
1323
nodes: (Node | Comment)[],
14-
) => {
15-
const ranges: { start: number; end: number }[] = nodes.filter(
24+
injectedCode?: string,
25+
injectIdx: number = 0,
26+
): string => {
27+
const ranges: (BoundedNodeOrComment | InjectedCode)[] = nodes.filter(
1628
(node): node is BoundedNodeOrComment => {
1729
const start = Number(node.start);
1830
const end = Number(node.end);
1931
return Number.isSafeInteger(start) && Number.isSafeInteger(end);
2032
},
2133
);
34+
if (injectedCode !== undefined) {
35+
ranges.push({
36+
type: 'InjectedCode',
37+
start: injectIdx,
38+
end: injectIdx,
39+
});
40+
}
2241
ranges.sort((a, b) => a.start - b.start);
2342

2443
const chunks: string[] = [];
2544
let idx = 0;
2645

27-
for (const { start, end } of ranges) {
46+
for (const { type, start, end } of ranges) {
2847
if (start > idx) {
2948
chunks.push(code.slice(idx, start));
3049
idx = start;
3150
}
51+
52+
if (injectedCode !== undefined && type === 'InjectedCode') {
53+
chunks.push(injectedCode);
54+
}
55+
3256
if (end > idx) {
3357
idx = end;
3458
}

src/utils/extract-ast-nodes.ts

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,21 @@
11
import { ParseResult } from '@babel/parser';
22
import traverse, { NodePath } from '@babel/traverse';
3-
import { Directive, File, ImportDeclaration } from '@babel/types';
3+
import { File, ImportDeclaration, Program } from '@babel/types';
44

55
export function extractASTNodes(ast: ParseResult<File>) {
66
const importNodes: ImportDeclaration[] = [];
7-
const directives: Directive[] = [];
7+
let injectIdx = 0;
88
traverse(ast, {
9-
Directive(path: NodePath<Directive>) {
10-
// Only capture directives if they are at the top scope of the source
11-
// and their previous siblings are all directives
12-
if (
13-
path.parent.type === 'Program' &&
14-
path.getAllPrevSiblings().every((s) => {
15-
return s.type === 'Directive';
16-
})
17-
) {
18-
directives.push(path.node);
19-
20-
// Trailing comments probably shouldn't be attached to the directive
21-
path.node.trailingComments = null;
9+
Program(path: NodePath<Program>) {
10+
/**
11+
* Imports will be injected before the first node of the body and
12+
* its comments, skipping InterpreterDirective and Directive nodes.
13+
* If the body is empty, default to 0, there will be no imports to
14+
* inject anyway.
15+
*/
16+
for (const node of path.node.body) {
17+
injectIdx = node.leadingComments?.[0]?.start ?? node.start ?? 0;
18+
break;
2219
}
2320
},
2421

@@ -31,5 +28,5 @@ export function extractASTNodes(ast: ParseResult<File>) {
3128
}
3229
},
3330
});
34-
return { importNodes, directives };
31+
return { importNodes, injectIdx };
3532
}

src/utils/get-code-from-ast.ts

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import generate from '@babel/generator';
2-
import { Directive, InterpreterDirective, Statement, file } from '@babel/types';
2+
import { Statement, file } from '@babel/types';
33

44
import { newLineCharacters } from '../constants';
55
import { PrettierOptions } from '../types';
6+
import { assembleUpdatedCode } from './assemble-updated-code';
67
import { getAllCommentsFromNodes } from './get-all-comments-from-nodes';
7-
import { removeNodesFromOriginalCode } from './remove-nodes-from-original-code';
88

99
/**
1010
* This function generate a code string from the passed nodes.
@@ -13,31 +13,19 @@ import { removeNodesFromOriginalCode } from './remove-nodes-from-original-code';
1313
*/
1414
export const getCodeFromAst = (
1515
nodes: Statement[],
16-
directives: Directive[],
1716
originalCode: string,
18-
interpreter?: InterpreterDirective | null,
17+
injectIdx: number = 0,
1918
options?: Pick<PrettierOptions, 'importOrderImportAttributesKeyword'>,
2019
) => {
2120
const allCommentsFromImports = getAllCommentsFromNodes(nodes);
2221

23-
const nodesToRemoveFromCode = [
24-
...directives,
25-
...nodes,
26-
...allCommentsFromImports,
27-
...(interpreter ? [interpreter] : []),
28-
];
29-
30-
const codeWithoutImportsAndInterpreter = removeNodesFromOriginalCode(
31-
originalCode,
32-
nodesToRemoveFromCode,
33-
);
22+
const nodesToRemoveFromCode = [...nodes, ...allCommentsFromImports];
3423

3524
const newAST = file({
3625
type: 'Program',
3726
body: nodes,
38-
directives,
27+
directives: [],
3928
sourceType: 'module',
40-
interpreter: interpreter,
4129
leadingComments: [],
4230
innerComments: [],
4331
trailingComments: [],
@@ -55,10 +43,13 @@ export const getCodeFromAst = (
5543
importAttributesKeyword: options?.importOrderImportAttributesKeyword,
5644
});
5745

58-
return (
46+
return assembleUpdatedCode(
47+
originalCode,
48+
nodesToRemoveFromCode,
5949
code.replace(
6050
/"PRETTIER_PLUGIN_SORT_IMPORTS_NEW_LINE";/gi,
6151
newLineCharacters,
62-
) + codeWithoutImportsAndInterpreter.trim()
52+
),
53+
injectIdx,
6354
);
6455
};

tests/ImportsNotSeparated/__snapshots__/ppsi.spec.js.snap

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,6 @@ const workletAdd = (a:number,b:number) => {
180180
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
181181
"use strict";
182182
"use client";
183-
184183
import abc from "@core/abc";
185184
import otherthing from "@core/otherthing";
186185

0 commit comments

Comments
 (0)