Working with remotes

Description

This week I continued my open source journey by contributing to the My-First-SSG. I started off creating an issue to implement a --config flag which would allow the users to supply options through a config file:
https://github.com/DerekJxy/My-First-SSG/issues/31

The idea was that users should be able to specify all of their SSG options in a JSON formatted configuration file instead of having to pass them all as command line arguments every time. After this change, the user would be able to:

# Option 1: use command line arguments (the way it was):
ssg --input ./site --output ./build --stylesheet https://cdn.jsdelivr.net/npm/water.css@2/out/water.css --lang fr

# Option 2: use a config file (new)
ssg --config ./ssg-config.json

Implementing and creating a PR

This part went smoothly for me and I didn't really face any challenges. Since this feature is new and basically just parses a config file instead of using the command line arguments, I didn't have to modify any of the existing code.

This feature should simply be a function call if the code was more modularized but since the code I was working on was already directly reading values from argv, I decided to copy any config options directly onto that object (to avoid touching other code).

      let fileContents = fs.readFileSync(argv.config);

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

      //Assigning options from config to argv (which is being used directly to generate HTML)
      argv.input = configData.input;
      argv.output = configData.output ? configData.output : './dist'; //Does not work due to Issue #12
      argv.lang = configData.lang ? configData.lang : 'en-CA';

I then wrapped these with some further validation for the config path and format so that appropriate error messages could be thrown:

if(argv.config){
  if(fs.existsSync(argv.config)){
    if(path.extname(argv.config).toLocaleLowerCase() == '.json'){
        //Read and parse config file
        //...
    }
    else {
      console.log("Config file must be JSON!", path.extname(argv.config));
      process.exit(-1);
    }
  }
  else {
    console.log("Config file missing!");
    process.exit(-1);
  }
}

With some testing, I was confident that this functionality was implemented as intended and I also made sure that it didn't break anything else (thankfully, it didn't!).
There was however an existing bug #12, due to which the output directory read from the config file wasn't being used. This was because the output dir was hardcoded to ./dist and I also tested to make sure that my changes were correctly reading the output option, so this was not related to my PR.

At this point, pushed my changes to the remote and created a pull request:
https://github.com/DerekJxy/My-First-SSG/pull/32/files

Reviewing cycle

The pr was directly approved and merged by the code owner earlier today, and there ended up being no back and forth in that regards.

Someone also created a similar issue on my project cli-ssg earlier this week to add support for a config JSON file and so I assigned it to them. There hasn't been a pull request as of me writing this blog, but I look forward to reviewing it!
https://github.com/dhillonks/cli-ssg/issues/17