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:
- Make a feature...
- ... but first, refactor this and that....
- ... 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
- Invalid module refactor
- Stale feature flags
- 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)