Refactoring existing code and rebasing

Background

I was looking at my static site generation tool, cli-ssg and being an open source repository, it had received a couple of commits from contributors. Now while the tool definitely worked as expected, I thought that this was a good moment to refactor the codebase to improve the structure and maintainability of the project.

Refactoring cli-ssg

I started off by creating a refactoring branch from main. I then spent some time looking in the project to find areas where I could improve the code quality.

When I had initiated this project, I decided to go with a module based approach even though at that time it only really supported .txt input. This drastically reduced the refactoring effort as a major portion of the code involving .html generation was already in a separate module called generateHtml.js.

That being said, index.js was looking a bit messy after the pull request for the --config option was merged into main (#18).

The way the options for the input path, output directory, language and stylesheet were being parsed and validated could definitely use some improvement and I decided to refactor the following code:

  .check((argv) => {

    //Input
    if(argv.i){

      if(!fs.existsSync(argv.i)){
        throw new Error("Input path must be a file or directory");
      }
    }

    //Output
    else if(argv.o != outputDir){
      if(fs.existsSync(argv.o)){
        if(!fs.lstatSync(argv.o).isDirectory()){
          throw new Error("Output path points to a file. Output directory must be valid")
        }
      }
      else throw new Error("Output directory must be valid");
    }

    //Config
    else if(argv.c){

      if(isJSON(process.argv[3])){

        if(!fs.existsSync(argv.c)){
          throw new Error("JSON file path does not exist");
        }
        var data = JSON.parse(fs.readFileSync(argv.c));

        if(data.input)      argv.i = data.input
        if(data.output)     argv.o = data.output
        if(data.stylesheet) argv.s = data.stylesheet
        if(data.lang)       argv.l = data.lang

      }else{
        throw new Error("The passed file should be of JSON format")
      }
    }
    else {
      throw new Error("A config file(.JSON) or an input file is required");
    }

    return true;
  })
  .argv;

Before I started refactoring, I noticed some problems with the code which were missed during code review:

No error handling when reading/parsing the JSON file
No validation for input, output options when they were parsed from JSON
An unhandled error would be thrown if an output directory path, which did not exist, was passed to the tool.

To fix these problems, I decided to refactor the option parsing and validation process into a new module. The class Options could then be used to store and process options for the tool such as:

input: Input file/folder to be processed
output: Output directory
lang: Language attribute for the html element
stylesheet: CSS stylesheet for the website

To store and process these, I wrote a class called Options and stored it in configOptions.js.

class Options{
  constructor(input, output, stylesheet, lang){
    this.input = input;
    this.output = output;
    this.stylesheet = stylesheet;
    this.lang = lang;
  }
  static get DEFAULT_OUTPUT(){ return './dist'; }
  static get DEFAULT_LANG(){ return 'en-CA'; }
}
module.exports.Options = Options;

Refactoring the config parsing functionality into this new module:

class Options{
   /**
   * Parses options using a file path
   * @param {string} filePath 
   * @return {Boolean} true if the options were parsed
   */
  parseConfig(filePath){
    if(fs.existsSync(filePath)){
      if(path.extname(filePath).toLocaleLowerCase() == '.json'){
        let fileContents = fs.readFileSync(filePath);
        let configData;

        try{
          configData = JSON.parse(fileContents);
        }
        catch(err){
          console.log('Error while parsing JSON: ', err);
          process.exit(-1);
        }

        this.input = configData.input;
        this.output = configData.output ? configData.output : Options.DEFAULT_OUTPUT;
        this.lang = configData.lang ? configData.lang : Options.DEFAULT_LANG;
        this.stylesheet = configData.s;

      }
      else {
        console.log("Config file must be JSON!", path.extname(filePath));
        process.exit(-1);
      }
    }
    else {
      console.log(`${filePath} not found! A JSON config file must be supplied`);
      process.exit(-1);
    }

    console.log(chalk.green(`Options successfully retrieved from ${filePath}`));
    return true;
  }
}

Adding methods to validate the options:

class Options{
  //Returns true if the input path is validated, otherwise throws an error
  validateInput(){
    if(!fs.existsSync(this.input)){
      throw new Error(`${this.input} does not exist. Input path must be a file or directory`);
    }
    return true;
  }

  //Returns true if the output path is validated, otherwise throws an error
  validateOutput(){
    if(this.output != Options.DEFAULT_OUTPUT){
      if(fs.existsSync((this.output))){
        if(!fs.lstatSync((this.output)).isDirectory()){
          throw new Error("Output path points to a file. Output directory must be valid")
        }
      }
      else throw new Error(`${this.output} does not exist. Output directory must be valid`);
    }
    return true;
  }
}

After I had written the Options module, it was now time to use it in index.js to parse and process the options supplied to the program. Starting off by importing the module:

const { Options } = require("./configOptions");

With the help of the Options module, I was able to simplify the messy index.js by reducing the options processing from 46 lines to just 12 lines:

.check((argv) => {
    options = new Options(argv.i, argv.o, argv.s, argv.l);

    if(argv.c){
      //Parse and use options from the config file
      options.parseConfig(argv.c);
    }

    //Validate the options
    return options.validateInput() && options.validateOutput();
  })
  .argv;

Since I was now using a variable called options to store the tool options, all I had to do was to change the arguments for .html generation:

generateHtml(options.input, options.output, options.stylesheet, options.lang);

And Voila! index.js was looking clean again and everything still worked just like before. The final change I made was to move generateHtml.js and configOptions.js to bin/modules

Organizing changes and rebasing

The refactoring work I mentioned above was done in the form of multiple separate commits. I had a total of 5 commits by the time I had finished:

Fixing the error handling bug introduced by the config feature

Refactor options validation and parsing into a new Options class

Move the HTML generation and tool options module to bin

Refactor paths for options and generate html modules

Improve linting and indentation

Before I merged these changes into main, I squashed them into a single commit using

git rebase main -i

I also ended up modifying its commit message to highlight the work done using git commit --amend.
At this point, I just performed a merge on the main branch to integrate the changes from my refactoring branch and pushed these changes to GitHub.

Final commit on main for refactoring work: b406d2f