Skip to content

Commit

Permalink
Update for new Babel, and remove workarounds
Browse files Browse the repository at this point in the history
  • Loading branch information
@fbartho
fbartho committed May 11, 2023
1 parent 2f300ef commit 8fcda28
Show file tree
Hide file tree
Showing 13 changed files with 83 additions and 152 deletions.
5 changes: 2 additions & 3 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,8 @@ Specific cases we handle:

- If you leave a gap after a comment at the top of your file, we will avoid moving it around if the imports below it shift.
- If you have comments that come after after your last import (with a gap); that comment and following code will stay below the imports.
- In general, if you place a single-line `CommentLine` (eg. `// …`) on the same line as an `ImportDeclaration` or `ImportSpecifier`, we will keep it attached to that same specifier if that line moves around (due to mergers, or sorting changes due to newly inserted imports).
- Multi-line-capable `CommentBlocks` (eg. `/** … */`) are automatically formatted to be on a new line by Prettier, so we do not change this behavior.
- Other comments are preserved, and are generally considered `leadingComments` for the subsequent `ImportDeclaration` or `ImportSpecifier`
- In general, if you place a single-line comment on the same line as an Import `Declaration` or `*Specifier`, we will keep it attached to that same specifier if that line moves around (due to mergers, or sorting changes due to newly inserted imports).
- Other comments are preserved, and are generally considered `leadingComments` for the subsequent Import `Declaration` or `*Specifier`

## FAQ / Troubleshooting

Expand Down
25 changes: 7 additions & 18 deletions src/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,29 +34,18 @@ export const TYPES_SPECIAL_WORD = '<TYPES>';
const PRETTIER_PLUGIN_SORT_IMPORTS_NEW_LINE =
'PRETTIER_PLUGIN_SORT_IMPORTS_NEW_LINE';

/** Use this to force a newline at top-level scope (good for newlines generated between import blocks) */
export const newLineNode = expressionStatement(
stringLiteral(PRETTIER_PLUGIN_SORT_IMPORTS_NEW_LINE),
);
export const injectNewlinesRegex = /"PRETTIER_PLUGIN_SORT_IMPORTS_NEW_LINE";/gi;

/**
* If you have a trailing single-line comment on an ImportSpecifier, and attached to
* the following ImportSpecifier you have a leading single-line comment,
* Then Babel's code-generator might render the two comments on the same line.
*
* Additionally, if you have a single-line-comment trailing an ImportDeclaration,
* babel might pull
*
* This probably should be reported upstream, but for now we have a workaround.
*/
export const PATCH_BABEL_GENERATOR_DOUBLE_COMMENTS_ON_ONE_LINE_ISSUE = true;

export const forceANewlineForImportsWithAttachedSingleLineComments = () => ({
/** Use this if you want to force a newline, but you're attaching to leading/inner/trailing Comments */
export const forceANewlineUsingACommentStatement = () => ({
type: 'CommentLine' as const,
value: 'PRETTIER_PLUGIN_SORT_IMPORTS_SINGLE_LINE_COMMENTS_PATCH',
value: 'PRETTIER_PLUGIN_SORT_IMPORTS_NEWLINE_COMMENT',
start: -1,
end: -1,
loc: { start: { line: -1, column: -1 }, end: { line: -1, column: -1 } },
});
export const forceANewlineForImportsWithAttachedSingleLineCommentsRegex =
/\/\/PRETTIER_PLUGIN_SORT_IMPORTS_SINGLE_LINE_COMMENTS_PATCH/gi;

export const injectNewlinesRegex =
/("PRETTIER_PLUGIN_SORT_IMPORTS_NEW_LINE";|\/\/PRETTIER_PLUGIN_SORT_IMPORTS_NEWLINE_COMMENT)/gi;
22 changes: 12 additions & 10 deletions src/utils/adjust-comments-on-sorted-nodes.ts
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
import {
addComments,
removeComments,
type ImportDeclaration,
} from '@babel/types';
import { removeComments, type ImportDeclaration } from '@babel/types';

import { ImportOrLine } from '../types';
import {
Expand All @@ -22,14 +18,20 @@ export const adjustCommentsOnSortedNodes = (
originalDeclarationNodes: ImportDeclaration[],
finalNodes: ImportOrLine[],
) => {
const outputDeclarationNodes: ImportDeclaration[] = finalNodes.filter(
const outputNodes: ImportDeclaration[] = finalNodes.filter(
(n) => n.type === 'ImportDeclaration',
) as ImportDeclaration[];
if (originalDeclarationNodes.length === 0 || outputNodes.length === 0) {
// Nothing to do, because there are no ImportDeclarations!
return finalNodes;
}

const registry = getCommentRegistryFromImportDeclarations(
outputDeclarationNodes,
originalDeclarationNodes[0],
);
const registry = getCommentRegistryFromImportDeclarations({
outputNodes,
firstImport: originalDeclarationNodes[0],
lastImport:
originalDeclarationNodes[originalDeclarationNodes.length - 1],
});

// Make a copy of the nodes for easier debugging & remove the existing comments to reattach them
// (removeComments clones the nodes internally, so we don't need to do that ourselves)
Expand Down
140 changes: 45 additions & 95 deletions src/utils/get-comment-registry.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ import {
type ImportDeclaration,
} from '@babel/types';

import {
forceANewlineForImportsWithAttachedSingleLineComments,
newLineNode,
PATCH_BABEL_GENERATOR_DOUBLE_COMMENTS_ON_ONE_LINE_ISSUE,
} from '../constants';
import { newLineNode } from '../constants';
import { ImportOrLine, ImportRelated, SomeSpecifier } from '../types';

const SpecifierTypes = [
Expand All @@ -26,7 +22,7 @@ function nodeId(node: Comment | ImportRelated): string {
}`;
}
if (node.type === 'CommentLine') {
return `${node.type}::${node.start}::${node.loc.start.line}`;
return `${node.type}::${node.start}::${node.loc?.start.line}`;
}
return `${node.type}::${node.start}`;
}
Expand Down Expand Up @@ -95,7 +91,8 @@ const attachCommentsToRegistryMap = <
comments,

owner,
firstImportDeclaration,
firstImport,
lastImport,
}: {
commentRegistry: Map<string, CommentEntry>;
/**
Expand All @@ -108,7 +105,10 @@ const attachCommentsToRegistryMap = <

owner: T;

firstImportDeclaration: ImportDeclaration;
/** Original declaration, not the re-sorted output-node! */
firstImport: ImportDeclaration;
/** Original declaration, not the re-sorted output-node! */
lastImport: ImportDeclaration;
}) => {
let commentCounter = 0;

Expand Down Expand Up @@ -140,18 +140,20 @@ const attachCommentsToRegistryMap = <
continue;
}

const currentOwnerIsFirstImport = nodeId(owner) === nodeId(firstImport);
const currentOwnerIsLastImport = nodeId(owner) === nodeId(lastImport);

const isSameLineAsCurrentOwner =
commentIsSingleLineType && // Prettier doesn't allow block comments to stay on same line as expressions
owner.loc?.start.line === comment.loc?.start.line;

// If there's a gap, but this is the first-import, don't treat this line as "attached to the next node"
// LeadingGap is used with firstImport to protect top-of-file comments, and pick the right ImportSpecifier when Specifiers are re-sorted
const hasLeadingGap =
nodeId(owner) === nodeId(firstImportDeclaration) &&
comment.loc?.start.line < (owner.loc?.start.line || 0) - 1;
(comment.loc?.start.line || 0) < (owner.loc?.start.line || 0) - 1;

// If it's not on the same line, don't treat this line as "attached to the previous node"
// TrailingGap is used with lastImport to protect bottom-of-imports comments, and pick the right ImportSpecifier when Specifiers are re-sorted
const hasTrailingGap =
comment.loc?.start.line > (owner.loc?.start.line || 0) + 1;
(comment.loc?.start.line || 0) > (owner.loc?.start.line || 0) + 1;

if (attachmentKey === 'trailingComments') {
// Trailing comments might be on the same line "attached"
Expand All @@ -171,7 +173,14 @@ const attachCommentsToRegistryMap = <
} else if (hasTrailingGap) {
// This comment is either a leading comment on the next node, or it's an unrelated comment following the imports
// Trailing comment, not on the same line, so either it will get attached correctly, or it will be dropped below imports
// [Intentional empty block]
if (currentOwnerIsLastImport) {
// [Intentional empty block] will automatically be attached from other nodes, or will fall to bottom
} else {
// This is a specifier or a non-last import.
commentEntry.processingPriority +=
DeferredCommentClaimPriorityAdjustment.trailingNonSameLine;
deferredCommentClaims.push(commentEntry);
}
} else {
// This comment should be kept close to the ImportDeclaration it follows
commentEntry.processingPriority +=
Expand All @@ -180,7 +189,7 @@ const attachCommentsToRegistryMap = <
}
continue; // Unnecessary, but explicit
} else if (attachmentKey === 'leadingComments') {
if (hasLeadingGap) {
if (currentOwnerIsFirstImport && hasLeadingGap) {
debugLog?.('Found a disconnected leading comment', {
comment,
owner,
Expand Down Expand Up @@ -236,12 +245,20 @@ const attachCommentsToRegistryMap = <
* Utility that walks ImportDeclarations and the associated comment nodes
* It returns a list of CommentEntry objects that tell you which nodes comments should be associated with
*/
export const getCommentRegistryFromImportDeclarations = (
outputNodes: ImportDeclaration[],
/** Original first import declaration, not the output-node! */
firstImportDeclaration: ImportDeclaration,
) => {
if (outputNodes.length === 0 || !firstImportDeclaration) {
export const getCommentRegistryFromImportDeclarations = ({
firstImport,
lastImport,
outputNodes,
}: {
/** Original declaration, not the re-sorted output-node! */
firstImport: ImportDeclaration;
/** Original declaration, not the re-sorted output-node! */
lastImport: ImportDeclaration;

/** Constructed Output Nodes */
outputNodes: ImportDeclaration[];
}) => {
if ((outputNodes.length === 0 || !firstImport, !lastImport)) {
return [];
}

Expand All @@ -264,16 +281,9 @@ export const getCommentRegistryFromImportDeclarations = (
specifierRegistry.set(specifier.loc?.start.line || 0, specifier);
});

// debugLog?.({
// specifierRegistry: [...specifierRegistry.values()].map((s) => ({
// line: s?.loc?.start.line,
// name: (s as any).imported,
// })),
// });

// Detach all comments, but keep their state.
// The babel renderer would otherwise move them around based on their original attachment.
// Register them in a specific order (inner, trailing, leading) so that they attach appropriately (ImportDeclarations)
// Register them in a specific order (inner, trailing, leading) so that the our best attachment gets priority

for (const attachmentKey of orderedCommentKeysToRegister) {
debugLog?.(
Expand All @@ -287,7 +297,8 @@ export const getCommentRegistryFromImportDeclarations = (
attachmentKey,
comments: Array.from(declarationNode[attachmentKey] || []),
owner: declarationNode,
firstImportDeclaration,
firstImport,
lastImport,
});

for (const specifierNode of declarationNode.specifiers) {
Expand All @@ -297,7 +308,8 @@ export const getCommentRegistryFromImportDeclarations = (
attachmentKey,
comments: Array.from(specifierNode[attachmentKey] || []),
owner: specifierNode,
firstImportDeclaration,
firstImport,
lastImport,
});
}
}
Expand Down Expand Up @@ -331,34 +343,15 @@ export const getCommentRegistryFromImportDeclarations = (
const targetAssociation = shouldPatchAssociation
? CommentAssociation.trailing
: entry.association;
if (shouldPatchAssociation) {
debugLog?.(
'Reattaching orphaned specifier comment to new owner',
{
old_line: line,
old_association: entry.association,
targetAssociation,
old_owner: entry.owner,
owner,
},
);
debugLog?.({
owner_loc: owner.loc?.start,
comment_loc: entry.comment.loc.start,
});
}

debugLog?.(
'Reattaching2',
'Reattaching',
id,
entry.association,
targetAssociation,
entry.comment.value,
{ hasNewOwner, owner, entry_owner: entry.owner },
);
if (entry.comment.value === ' h3 leading single-line-comment') {
debugLog?.(entry.comment);
}

commentRegistry.set(id, {
...entry,
Expand Down Expand Up @@ -431,54 +424,11 @@ export function attachCommentsToOutputNodes(
(commentCollection as Comment[]).push(comment);
}

// console.debug(
// 'outputNodes!',
// outputNodes.map((n) =>
// (n as any).specifiers?.map((s: SomeSpecifier) => ({
// name: s.imported?.name,
// leading: JSON.stringify(s.leadingComments),
// trailing: JSON.stringify(s.trailingComments),
// })),
// ),
// );

if (PATCH_BABEL_GENERATOR_DOUBLE_COMMENTS_ON_ONE_LINE_ISSUE) {
outputNodes
.filter((n) => n.type === 'ImportDeclaration')
.map((n) => {
// Patching up another bug: babel will pull up a single-line trailing-comment
// that follows an ImportDeclaration even if it wasn't supposed to be on the same line
const trailingComments = n.trailingComments;
if (
Array.isArray(trailingComments) &&
trailingComments.length >= 1
) {
if (
trailingComments[0].type === 'CommentLine' &&
trailingComments[0].loc?.start.line !== n.loc?.end.line
) {
// This comment is on a different line, let's make sure there's a newline
trailingComments.unshift(
forceANewlineForImportsWithAttachedSingleLineComments(),
);
}
}
return n;
})
.map((n) => (n as ImportDeclaration).specifiers || [])
.flat()
.forEach((s) => {
(s.leadingComments as Comment[])?.unshift(
forceANewlineForImportsWithAttachedSingleLineComments(),
);
});
}

if (Array.isArray(outputNodes[0].leadingComments)) {
if (outputNodes[0].leadingComments.length > 0) {
// Convert this to a newline node!
outputNodes[0] = {
...newLineNode,
...newLineNode, // Inject a newline after top-of-file comments
leadingComments: outputNodes[0].leadingComments,
};
} else {
Expand Down
6 changes: 3 additions & 3 deletions tests/Flow/__snapshots__/ppsi.spec.ts.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export function givesAFoo3Obj(): AliasFoo3 {
/**
* @flow
*/
// I am top level comment in this file.
import thirdParty from "third-party";
Expand Down Expand Up @@ -131,7 +130,6 @@ export function givesAFoo3Obj(): AliasFoo3 {
/**
* @flow
*/
// I am top level comment in this file.
import thirdDisco0 from "third-disco0";
Expand Down Expand Up @@ -198,6 +196,8 @@ import {fooValue} from './foo';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/**
* @flow
*/ import { fooValue, type Foo } from "./foo";
*/
import { fooValue, type Foo } from "./foo";
`;

0 comments on commit 8fcda28

Please sign in to comment.