Typescript: Recursion file-system lookup

Question

Previously I asked a question to find the best practice of recursively running a function to retrieve all import/export statements of files in NodeJs here.

After some discussion I'm not satisfied yet, so I'm going to post my rewrote code from what I learned in previous discussion in hope of finding a better approach or simply getting review of my code.

I'm calling preprocessTSFiles function which runs getFiles with the entry-point and return all its import/export statements plus itself which leads to a recursive call of getImporters from VinylFile class.

preprocess-files.ts

import * as path from 'path';
import * as vinyl from 'vinyl';
import { VinylFile } from './file';
import {
  ensureMakeDir,
  isArray,
  isDirectory,
  isEmpty,
  isFile,
  isNil,
  readFile,
  writeFile,
} from './utils';

import { InlineStyles } from './plugins/inline-styles';
import { InlineTemplate } from './plugins/inline-template';

export interface HandlerPlugin {
  (file: VinylFile): Promise<string>;
}

export async function preprocessTSFiles(entryFilePath: string, destDir: string, baseDir?: string): Promise<void> {
  const fileList: string[] = [];
  const plugins = [InlineTemplate, InlineStyles];

  if (isNil(baseDir)) {
    baseDir = path.dirname(entryFilePath);
  }

  const allFiles = await getFiles(entryFilePath, []);

  for (const file of allFiles) {
    for (const plugin of plugins) {
      const newContent = await plugin(file);

      if (newContent != file.content) {
        file.setContent(newContent);
      }
    }

    const currentPath = file.filePath;
    const destPath = currentPath.replace(baseDir + path.sep, '');
    const absDestPath = path.resolve(destDir, destPath);

    file.copyTo(absDestPath);
  }
}

async function getFiles(entryFilePath: string, excludeList: string[]): Promise<VinylFile[]> {
  const entryFile = new VinylFile(entryFilePath);
  const excludeFromList: string[] = [entryFilePath];
  return [entryFile, ...await entryFile.getImporters(excludeFromList)];
}

file.ts

import * as path from 'path';
import {
  ensureMakeDir,
  isDirectory,
  isFile,
  isNil,
  readFile,
  writeFile,
} from './utils';

export class VinylFile {
  private _content: string;
  private importStatementRE = /(?:import|export)\s*(?:(?:\{[^}]*\}|\*|\w+)(?:\s*as\s+\w+)?(?:\s+from)?\s*)?([`'"])((?:\\[\s\S]|(?!\1)[^\\])*?)\1/ig;

  public dirPath: string;

  constructor(public filePath: string) {
    this.dirPath = path.dirname(filePath);
  }

  get content(): string {
    if (isNil(this._content)) {
      this._content = readFile(this.filePath);
    }

    return this._content;
  }

  public setContent(content: string): void {
    this._content = content;
  }

  public copyTo(destFilePath: string): void {
    const dirPath = path.dirname(destFilePath);
    ensureMakeDir(dirPath);
    writeFile(destFilePath, this.content);
  }

  public hasImporters(): boolean {
    this.importStatementRE.lastIndex = 0;
    return this.importStatementRE.test(this.content);
  }

  public async getImporters(excludeFrom?: string[]): Promise<VinylFile[]> {
    const importees: VinylFile[] = [];
    const importStatementMatchIndex = 2;
    let importersMatch: RegExpMatchArray | null;

    if (isNil(excludeFrom)) {
      excludeFrom = [];
    }

    // Reset current index to 0
    // RegEx.test moves this into 1
    this.importStatementRE.lastIndex = 0;
    while ((importersMatch = this.importStatementRE.exec(this.content)) && !isNil(importersMatch)) {
      const importee = importersMatch[importStatementMatchIndex];
      const resolvedPath = path.resolve(this.dirPath, importee);

      if (isDirectory(resolvedPath)) {
        const resolvedIndexFilePath = path.resolve(resolvedPath, 'index.ts');

        if (isFile(resolvedIndexFilePath) && excludeFrom.indexOf(resolvedIndexFilePath) < 0) {
          const resolvedIndexFile = new VinylFile(resolvedIndexFilePath);
          excludeFrom.push(resolvedIndexFilePath);
          importees.push(resolvedIndexFile);

          if (resolvedIndexFile.hasImporters()) {
            importees.push(...await resolvedIndexFile.getImporters(excludeFrom));
          }
        }
      } else {
        let resolvedFilePath = resolvedPath;
        if (path.extname(resolvedFilePath) !== '.ts') {
          resolvedFilePath += '.ts';
        }

        if (isFile(resolvedFilePath) && excludeFrom.indexOf(resolvedFilePath) < 0) {
          const resolvedFile = new VinylFile(resolvedFilePath);
          excludeFrom.push(resolvedFilePath);
          importees.push(resolvedFile);

          if (resolvedFile.hasImporters()) {
            importees.push(...await resolvedFile.getImporters(excludeFrom));
          }
        }
      }
    }
    return importees;
  }
}

Answer 1

I see your code is more readable after the refactoring.

You had choose better name that clarified the intention. This is a good part.

I think the thought point is the while part.

In a functional fashion you could improve the code putting the matching part in a method:

getImportStatements() {
  const importStatementMatchIndex = 2;
  const importStatements = [];
  let importersMatch = this.importStatementRE.exec(this.content);
  while(!isNil(impotersMatch)) {
    importStatements.push(impotersMatch[importStatementMatchIndex]);
    importersMatch = this.importStatementRE.exec(this.content);
  }
  return importStatements;
}

This will return just a list with the matching strings. The reason for this change is to try to split up the responsabilities in a way that your code will be much easy to read and change.

This method isolate the logic on how you get the list of imports on the source file.

Then you could rewrite the main method removing the while loop and adding the following:

this.getImportStatements()
  .map((importee) => (path.resolve(this.dirPath, importee)))
  .map((resolvedPath) => {
    if (isDirectory(resolvedPath)) {
      return path.resolve(resolvedPath, 'index.ts');
    }
    return (path.extname(resolvedPath) !== '.ts') ? resolvedPath += '.ts' : resolvedPath;
  })
  .filter((resolvedFilePath) => (!isFile(resolvedFilePath) || excludeFrom.indexOf(resolvedFilePath) >= 0))
  .forEach((resolvedFilePath) => {
    const resolvedFile = new VinylFile(resolvedFilePath);
    excludeFrom.push(resolvedFilePath);
    importees.push(resolvedFile);

    if (resolvedFile.hasImporters()) {
      importees.push(...await resolvedFile.getImporters(excludeFrom));
    }
  });

The principle is to attach on the array returned from getImportStatements(), that guarantee to return at least an empty array.

Then it use the javascript array methods to map the strings to full path, filter and then collect.

Fell free to change the arrow functions in methods to increase the readability of the code.