fix/import-section-comments-removed

BREAKING_CHANGES.md

@@ -0,0 +1,11 @@
+# Import conductor V2
+
+#### Config Options Changes:
+
+- `autoMerge` option replaced with `noAutoMerge`, and the option default value is `false`.
+- `silent` option replaced with `verbose`, and the option default value is `false`.
+- The `-v` option alias now stands for the `verbose` option instead of getting the import conductor version.
+- The `-m` option alias (which used for the `autoMerge`) was removed.
+- The `-d` option alias now stands for the `dryRun` option instead of `autoAdd`.
+
+See the updated config options in [this](https://github.com/kreuzerk/import-conductor#options) section of the documentation.

__tests__/optimize-imports-mocks.ts

@@ -0,0 +1,57 @@
+export type testCase = { input: string; expected: string };
+
+export const readmeExample: testCase = {
+  input: `import fs from 'fs';
+import { CustomerService } from './customer.service';
+import { Customer } from './customer.model';
+import { Order } from '../order/order.model';
+import { Component, OnInit } from '@angular/core';
+import { LoggerService } from '@myorg/logger';
+import { Observable } from 'rxjs';
+import { spawn } from 'child_process';`,
+  expected: `import { Component, OnInit } from '@angular/core';
+import { spawn } from 'child_process';
+import fs from 'fs';
+import { Observable } from 'rxjs';
+
+import { LoggerService } from '@myorg/logger';
+
+import { Order } from '../order/order.model';
+
+import { Customer } from './customer.model';
+import { CustomerService } from './customer.service';`,
+};
+
+export const comments: testCase = {
+  input: `// file level comments shouldn't move
+import fs from 'fs';
+import { CustomerService } from './customer.service';
+import { Customer } from './customer.model';
+// should be above this import
+import { Order } from '../order/order.model';
+import { Component, OnInit } from '@angular/core';
+/* I will follow LoggerService wherever he goes */
+import { LoggerService } from '@myorg/logger';
+/**
+* important comment about Observables  
+*/
+import { Observable } from 'rxjs';
+import { spawn } from 'child_process';`,
+  expected: `// file level comments shouldn't move
+import { Component, OnInit } from '@angular/core';
+import { spawn } from 'child_process';
+import fs from 'fs';
+/**
+* important comment about Observables  
+*/
+import { Observable } from 'rxjs';
+
+/* I will follow LoggerService wherever he goes */
+import { LoggerService } from '@myorg/logger';
+
+// should be above this import
+import { Order } from '../order/order.model';
+
+import { Customer } from './customer.model';
+import { CustomerService } from './customer.service';`,
+};

__tests__/optimize-imports.spec.ts

@@ -2,6 +2,7 @@ import { actions, optimizeImports } from '@ic/conductor/optimize-imports';
 import * as config from '@ic/config';
 import fs from 'fs';
 import { Config } from '@ic/types';
+import { readmeExample, comments, testCase } from './optimize-imports-mocks';
 
 jest.mock('fs');
 
@@ -18,43 +19,37 @@ describe('optimizeImports', () => {
     thirdPartyDependencies: new Set<string>(['@angular/core', 'rxjs']),
   };
 
-  const readmeExample = `import fs from 'fs';
-import { CustomerService } from './customer.service';
-import { Customer } from './customer.model';
-import { Order } from '../order/order.model';
-import { Component, OnInit } from '@angular/core';
-import { LoggerService } from '@myorg/logger';
-import { Observable } from 'rxjs';
-import { spawn } from 'child_process';`;
-
-  const expectedResult = `import { Component, OnInit } from '@angular/core';
-import { spawn } from 'child_process';
-import fs from 'fs';
-import { Observable } from 'rxjs';
-
-import { LoggerService } from '@myorg/logger';
-
-import { Order } from '../order/order.model';
-
-import { Customer } from './customer.model';
-import { CustomerService } from './customer.service';`;
-
   beforeEach(() => {
     spyOn(config, 'getConfig').and.returnValue(basicConfig);
     (fs.existsSync as any).mockReturnValue(true);
+    (fs.writeFileSync as any).mockClear();
   });
 
-  it('should work with a basic example', async () => {
-    (fs.readFileSync as any).mockReturnValue(Buffer.from(readmeExample));
+  async function assertConductor({ expected, input }: testCase) {
+    (fs.readFileSync as any).mockReturnValue(Buffer.from(input));
     const file = 'test.ts';
-    await optimizeImports(file);
-    expect(fs.writeFileSync).toHaveBeenCalledWith(file, expectedResult);
+    const result = await optimizeImports(file);
+    expect(fs.writeFileSync).toHaveBeenCalledWith(file, expected);
+
+    return result;
+  }
+
+  it('should work with a basic example', async () => {
+    await assertConductor(readmeExample);
+  });
+
+  it('should work with comments', async () => {
+    await assertConductor(comments);
   });
 
   it('should skip the file when skip comment exists', async () => {
-    (fs.readFileSync as any).mockReturnValue(Buffer.from('// import-conductor-skip'));
-    const file = 'test.ts';
-    const result = await optimizeImports(file);
-    expect(result).toBe(actions.skipped);
+    const testCases = ['//import-conductor-skip', '// import-conductor-skip', '/* import-conductor-skip*/', '/*import-conductor-skip */'];
+    for (const testCase of testCases) {
+      (fs.readFileSync as any).mockReturnValue(Buffer.from(testCase));
+      const file = 'test.ts';
+      const result = await optimizeImports(file);
+      expect(result).toBe(actions.skipped);
+      expect(fs.writeFileSync).not.toHaveBeenCalled();
+    }
   });
 });

jest.config.js

@@ -11,7 +11,7 @@ module.exports = {
       tsConfig: 'tsconfig.spec.json',
     },
   },
-  testRegex: '(/__tests__/.*|(\\.|/)(test|spec))\\.tsx?$',
+  testRegex: '/__tests__/.*(test|spec)\\.tsx?$',
   moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'],
   moduleNameMapper: pathsToModuleNameMapper(compilerOptions.paths, { prefix: '<rootDir>/' }),
   testPathIgnorePatterns: ['<rootDir>/bin/help-menu.ts', '<rootDir>/bin/pretty-html-log.bin.ts'],

package.json

@@ -12,7 +12,7 @@
     "copy:readme": "copyfiles ./README.md ./dist",
     "format:write": "prettier --write 'index.ts'",
     "test": "jest",
-    "start": "ts-node ./src/index.ts playground/**/*.ts -v --dryRun -p '@custom' -i playground/**/*x.ts main.ts",
+    "start": "ts-node ./src/index.ts -s playground/sampleDir/main.ts -p '@custom'",
     "start:staged": "ts-node ./src/index.ts --staged",
     "start:bar": "ts-node ./src/index.ts --dryRun -s playground/sampleDir/bar.ts",
     "release": "semantic-release",

playground/sampleDir/main.ts

@@ -1,11 +1,17 @@
-import { sync } from 'glob';
-import { isVariableDeclaration } from 'typescript';
-
-import { FooModule } from '../someModule';
-import { enableProdMode } from '@custom/something';
-
-import { environment } from './environments/environment';
-import { SomeModule } from './someModule';
+// file level comments shouldn't move
+import fs from 'fs';
+import { CustomerService } from './customer.service';
+import { Customer } from './customer.model';
+// should be above this import
+import { Order } from '../order/order.model';
+import { Component, OnInit } from '@angular/core';
+/* I will follow LoggerService wherever he goes */
+import { LoggerService } from '@myorg/logger';
+/**
+ * important comment about Observables
+ */
+import { Observable } from 'rxjs';
+import { spawn } from 'child_process';
 
 if (environment.production) {
   enableProdMode();

src/conductor/get-import-statement-map.ts

@@ -2,7 +2,6 @@ import ts from 'typescript';
 
 import { getConfig } from '../config';
 
-import { collectImportStatement } from './collect-import-statements';
 import { mergeImportStatements } from './merge-import-statements';
 
 export function getImportStatementMap(importNodes: ts.Node[]): Map<string, string> {
@@ -11,7 +10,10 @@ export function getImportStatementMap(importNodes: ts.Node[]): Map<string, strin
 
   importNodes.forEach((node: ts.Node) => {
     const importSegments = node.getChildren();
-    const completeImportStatement = collectImportStatement(importSegments);
+    let completeImportStatement = node.getFullText();
+    if (completeImportStatement.startsWith('\n')) {
+      completeImportStatement = completeImportStatement.replace('\n', '');
+    }
     const importLiteral = importSegments.find((segment) => segment.kind === ts.SyntaxKind.StringLiteral)?.getText();
 
     if (!importLiteral) {

src/conductor/optimize-imports.ts

@@ -19,19 +19,35 @@ export const actions = {
   reordered: 'reordered',
 };
 
+function getFileComment(fileContent: string): string {
+  const fileComment = /^(?:(\/\/.*)|(\/\*[^]*?\*\/))/.exec(fileContent);
+  if (fileComment) {
+    const [singleLine, multiLine] = fileComment;
+    return singleLine || multiLine;
+  }
+
+  return '';
+}
+
 export async function optimizeImports(filePath: string): Promise<string> {
   // staged files might also include deleted files, we need to verify they exist.
   if (!/\.tsx?$/.test(filePath) || !existsSync(filePath)) {
     return actions.none;
   }
 
-  const fileContent = readFileSync(filePath).toString();
+  let fileContent = readFileSync(filePath).toString();
   const { staged, autoAdd, dryRun } = getConfig();
   if (/\/[/*]\s*import-conductor-skip/.test(fileContent)) {
     log('gray', filePath, 'skipped (via comment)');
     return actions.skipped;
   }
 
+  let fileComment = getFileComment(fileContent);
+  if (fileComment) {
+    fileContent = fileContent.replace(fileComment, '');
+    fileComment += '\n';
+  }
+
   const rootNode = ts.createSourceFile(filePath, fileContent, ts.ScriptTarget.Latest, true);
   const importNodes = collectImportNodes(rootNode);
   const importStatementMap = getImportStatementMap(importNodes);
@@ -42,12 +58,12 @@ export async function optimizeImports(filePath: string): Promise<string> {
 
   const categorizedImports = categorizeImportLiterals(importStatementMap);
   const sortedAndCategorizedImports = sortImportCategories(categorizedImports);
-  let result = formatImportStatements(sortedAndCategorizedImports);
+  const result = formatImportStatements(sortedAndCategorizedImports);
 
   const lastImport = importNodes.pop();
   const contentWithoutImportStatements = fileContent.slice(lastImport.end);
 
-  const updatedContent = `${result}${contentWithoutImportStatements}`;
+  const updatedContent = `${fileComment}${result}${contentWithoutImportStatements}`;
   const fileHasChanged = updatedContent !== fileContent;
   if (fileHasChanged) {
     !dryRun && writeFileSync(filePath, updatedContent);