DEV Community

Lukasz Ostrowski for Saleor Commerce

Posted on

Cleaning up large frontend codebase

Recently I started work on the new extensions functionality in Saleor Dashboard. This repo is quite large (450k LOC). I decided to make some cleanup before I introduce the feature.

So the plan was:

  1. Make a feature...
  2. ... but first, refactor this and that....
  3. ... but first, remove some dead code.

tldr: This post is about removing approx 30k LOC (~6.6%) of the codebase and bundle size (pre-gzipped) lowered by 350KB

Diagnosis

First of all, I have diagnosed where the main gains are. I found three main areas

  1. Invalid module refactor
  2. Stale feature flags
  3. Unused graphQL queries

I wanted to focus mainly on them, because I already had some understanding what's going on. I also wanted to keep them specifically, to reduce the code complexity before my change.

Invalid module refactor

Some time ago Saleor consolidated extensions model - we have merged Apps, Plugins and Webhooks under the single domain - extensions.

The refactor happened using feature flag, but it was not "branching" with specific differences (like show route A or B). Instead entire module was copy pasted 1:1 to a new directory. Then for few months we maintained both of them and they slowly started to differ.

The goal is to drop the old code without breaking anything

Stale feature flags

We also had several feature flags, all of them quite stale. All of them enabled, but entire code branching for disabled flags still was in the source. The goal here was to drop flags and remove dead code.

Unused queries

When I removed flags, I realised many graphQL queries exist in the codebase (and are processed by codegen into types and executable operations). The goal here was to remove as many of them as possible

Techniques

I will not write a deep dive of my every move in this process, but rather share some learnings on the process

Static analysis

During the cleanup I was using existing static analysis tool and improved their config. It's probably the best possible way to diagnose and maintain quality around dead code.

  • ESLint - detect unused declarations, plugins like graphQL plugin can help detecting issues with queries, etc.
  • Knip - scans the code for unused exports. Not always working, but it can show not-so-easy dependencies. For example, when code is imported by test, technically this code is not "unused". Knip can detect that
  • Dependency cruiser - can enforce architecture decisions like forbid circular deps etc

I was using such tools on every PR to validate my changes and find new issues

Tests

Obviously before the refactor, we should have good tests coverage to verify if our changes didn't break anything.

With AI this is easier than ever. Code that is hard to test is often not tested. Claude wrote tests for me, I only reviewed if assertions are valid.

Dropping default export and barrel files

Both export default and barrel files (index.js/ts) are known anti-patterns. Their existence make static analysis a nightmare. I did several runs to remove them before moving with other changes

Small PRs

It's tempting to add more and more changes into the same PR, but it never works.

  • Large PR is hard/impossible to be checked by human properly
  • LLMS context is too low to review them as well
  • It's not friendly to ask someone to check such code

Instead, I tried (not always successfully, but it's something) to introduce small and cohesive changes:

  • PR 1: Remove export default statements and update imports → reviewer only has one "context" to verify and usually if it builds, it works
  • PR 2: Remove barrel files and update imports → ditto
  • PR 3: Remove bunch of files, nothing else

etc.

I broke this rule once - and on the review the bug was found. I didn't have time to fix it then, and the change was quite large. Before I was able to go back to my work, I had so many conflicts, I had to start from scratch

AI codemods instead of direct changes

I realized using AI that for such a large codebase, AI often fail to keep the context. I use it to write a test suite, but I can't ask it to refactor the codebase.

What I did instead was starting to use AI to write codemods and other scripts.

For example, I haven't found a tool that would find unused graphQL queries (maybe because we colocate them in .ts files?). It's not easy to find dead code here, because codegen is transforming them to documents, hooks etc. So only way to find it is

  • Remove document, rebuild codegen, build app/types and check if it's failing
  • Automate it somehow

To automate it, I gave Claude rules how codegen is generating queries (eg query ProductsPage generates useProductsPage hook). It took approx 30 minutes, but it vibecoded a script that pretty much worked. Script itself can be a trash that I won't maintain, but I can use it to find what to remove, and delete it afterwards.

Keep flags clean

Our flags are controlled custom way, so we don't have any fancy tools to control their staleness. But we should, either use a service or introduce a process that will allow us to periodically drop them. Flags usually add major complexity and should be short living.

Top comments (0)