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
Top comments (0)