Codebase Health Best Practices

The SHOP team in the last year has seen major changes including an entirely new team of developers as well as a reset of our development standards and methods for maintaining codebase health. In this post, co-written by two developers on the SHOP team, we’ll be providing a high-level overview on development practices we’ve implemented in order to maintain the health of the project codebase.

SHOP Squad Development Standards
PR Review Process
There are many different approaches for how to best conduct code reviews. In 2023, the SHOP team may need to determine an agreed upon methodology for conducting code reviews on PRs originating from both internal and contract developers. However, currently the SHOP team adheres to the following rules when conducting code reviews.

1. All required changes will be marked in the PR. All comments need to be resolved before a PR is merged into the FMIC codebase.
2. If a PR has been created but is not ready to be merged into the FMIC codebase, the PR needs to be in draft status.
3. The PR should pass Acceptance Criteria stated in the Jira Ticket. Please leave notes in the PR for any additional changes or developer issues to be addressed.
4. The PR will not be reviewed if all tests did not pass after creating the PR.

Ensuring that all code is thoroughly reviewed before merging PRs into the codebase helps us prevent any bugs from making their way to production. When reviewing the code, the internal developers also check to ensure that the standards listed below were followed when developing new site features.

JavaScript and CSS Standards
When writing CSS, like many other developers, we chose to adhere to the Airbnb CSS/Sass Style Guide. Airbnb’s CSS/SASS Style Guide is considered by many developers to be a gold-standard approach to writing CSS. Their CSS formatting approach encourages a combination of OOCSS and BEM in order to help developers write code which is reusable, scalable, allows for less nesting, and a lesser amount of specificity when writing CSS.

When writing JavaScript, we also chose to adhere to Airbnb’s JavaScript Style Guide. Their JavaScript Style Guide includes simple, yet well-documented, best practices to follow when writing JavaScript. Additionally, as a team we determined that we would also adhere to the following rules when writing JavaScript.

1. Minimize jQuery use as much as possible. If the PR includes the use of jQuery instead of vanilla/plain JavaScript, state the reason why in the PR description.
2. When selecting one or more elements using client-side JavaScript, always use a CSS selector targeting a data attribute. This ensures that any client-side JavaScript targeting that specific element can be easily found within the codebase. Additionally, this ensures that classes are used specifically for CSS which generally allows for less confusion when searching through the codebase.

HTML
<button class=”btn-primary” type=”button” data-example-attribute>This is a Button</button>

JavaScript
const exampleButton = document.querySelector(“[data-example-attribute]”);

ISML Templates
Additionally, when reviewing PRs which include brand cartridges other than our main cartridge used for Fender.com (PreSonus, Jackson, and any other Fender brands which will be added to SFCC in the future) internal developers also review ISML templates to ensure that duplicate files are not being unnecessarily added to the codebase. Maintaining only the necessary amount of ISML templates helps the team create new site features for all SFCC sites much faster, as changes do not need to be duplicated amongst multiple brand templates.

Unit Testing
This year, the SHOP team adopted the practice of writing unit tests for new and existing code. While we knew this would be the start of a long and painstaking process, we knew that we could never accurately monitor the health of the codebase without unit testing our code. Due to the amount of unit tests needed to significantly increase our level of code coverage, the initiatives for unit testing on the SHOP team were broken down into three categories.

• Creating new unit tests for all newly created custom server-side functions.
• Backfilling unit tests by modifying the unit tests in the SalesforceCommerceCloud/storefront-reference-architecture repo and modifying them to work with our modified files in org_fmic.
• Backfilling unit tests that target our business critical site functionality.

Salesforce does include a suite of unit tests in their Github repo containing the Storefront Reference Architecture (SFRA). However, much of the core site functionality has been overwritten in our custom cartridge org_fmic. This being the case, we could not simply copy and paste the unit tests included in the SFRA repo and expect all tests to pass. While these included unit tests gave us a starting point to work off of, most of these tests will be modified to work with the code in org_fmic modified from app_storefront_base. This practice of modifying existing unit tests benefited our team quite a bit by providing the developers experience working with unit tests without having to write them from scratch. Once the SHOP developers gained some experience backfilling unit tests, this provided us with the necessary foundation to write unit tests from scratch for all newly created server-side functions.

Anyone who has written unit tests before knows that one of the most labor intensive aspects of unit testing is mocking out all objects and services the code being unit tested relies on. For this we are able to utilize the SalesforceCommerceCloud/dw-api-mock repository. This repo contains mocks for many Demandware classes and objects which are heavily used within the project. Utilizing this repo has helped us make strides building up our suite of unit tests to provide better code coverage. While there is still much work to be done in order to backfill all necessary unit tests, our team has made great process on this initiative in the past year.

Developer Unit Testing Resources

• https://fender.udemy.com/course/nodejs-unit-testing-in-depth/
• https://sinonjs.org/releases/v13/
• https://github.com/thlorenz/proxyquire
• https://mochajs.org/

Addition of Linting back into our SFCC Repository

Another important component of codebase health that was examined this year was linting. First, a quick introduction to linting. A Linter is a very practical tool for automated analysis of your source code looking for stylistic errors and bugs. A Linter assists in making sure that your code does not contain structural and functional issues which can in turn make your code much harder to maintain. In doing so a linter is a simple static code analyzer that basically flags bugs in your code from syntax errors. Below We’ve listed a few more reasons why you might want to add linting to your own codebase.

1. Gives you warnings when code may not be intuitive
2. Provides suggestions for common best practices
3. It keeps a consistent code style within a development team and therefore saves time during code reviews
4. Linting helps you reduce errors and improve the quality of your code
5. Linting improves code readability
6. Autofix feature can automatically correct many mistakes

As SFCC repositories in general tend to be very large bodies of code it is incredibly important to keep the code super maintainable and to automate processes that might save time in doing so. Here on the shop squad, we have a small and functional team of three developers, with additional contributions from external developers. Linting can help these external developers assimilate into our team's best practices and speed up PR reviews when they submit their work.

At the beginning of the year linting all our JavaScript files had been essentially turned off by having all the major directories of our project listed in the .eslintignore. This was done because we had been running our project on an outdated node version and the eslint-loader package had been deprecated that didn’t allow all of our files to be properly linted without erroring.

As eslint-loader is deprecated we found the suitable replacement to be migrating to the eslint-webpack-plugin. This work was completed in our webpack.js.config file. Below is what our old file looked like.

const config = {
  module: {
    rules: [
      {
        test: /\.js$/,
        exclude: /node_modules/,
        loader: 'eslint-loader',
        options: {
          // eslint options (if necessary)
        },
      },
    ],
  },
}

After migration to the new package

const ESLintPlugin = require('eslint-webpack-plugin');

const config = {
  plugins: [new ESLintPlugin(options)],
}

Upgrading this package now allowed us to lint all the JavaScript in the codebase and fully utilize the eslint package. An important feature of this package is its ability to auto-fix many of the linting errors in our project.

Fixing problems:
  --fix                           Automatically fix problems
  --fix-dry-run                   Automatically fix problems without saving the changes to the file system
  --fix-type Array                Specify the types of fixes to apply (directive, problem, suggestion, layout)

Using these flags we were able to automatically fix linting errors in each cartridge of our project using the command below. Ultimately fixing about 90% of our linting errors.

npx eslint --fix allourdirectories

Next was to add linting into our build process. To do this we edited the options in the new plugin package that was installed.

plugins: [
            new ESLintPlugin({emitWarning: true,
                emitError: false,
                failOnError: false,
                failOnWarning: false,
                quiet: true})
        ]

Finally, we will discuss our project linting plans moving forward. While right now we are just emitting warnings in the build about the linting errors but not failing the build. Our goal is to continue to work to manually fix the rest of the linting issues as we navigate throughout the repo working on different features. When that point is reached we will fail the build when a linting error occurs so our linting standards can be strictly enforced throughout the platform no matter the developer.

Maintaining codebase health is a necessity not only to prevent bugs from creeping into production, but also in order to maintain a scalable platform which can support the rapid development of new site features. In order to achieve this, the SHOP team added linting standards back into our eCommerce SFCC Repository, improved processes surrounding code reviews and managing internal and contract developer PRs, implemented JavaScript and CSS developer standards, and began eliminating and the enforcement of limiting unnecessary and duplicate ISML templates. As the SHOP team enters 2023, we feel confident about our ability to tackle Product initiatives while also maintaining and enforcing codebase health best practices.

-Coauthored by Dylan Crawshaw and Gabi Wethor