DEV Community: Sergey

You Don't Modularise a Frontend by Moving Files

Sergey — Sat, 29 Aug 2026 10:23:25 +0000

Notes from trying to give one React component a place it could actually call its own

We wanted to split a large frontend into packages.

This sounded, at first, like a file-moving problem.

We had a component. Let's call it WeatherCard.

It showed the current weather, allowed the user to save a location, sent analytics events, behaved slightly differently on the home page and in search results, participated in an experiment, and knew whether the application was running on mobile.

Nothing particularly exotic.

It lived here:

src/
  feed/
    cards/
      weather/
        WeatherCard.tsx

We wanted it here:

packages/
  weather-card/

So we moved it.

That took an afternoon.

Modularising it took considerably longer.

The files moved. The application came with them.

The first package looked promising.

packages/
  weather-card/
    WeatherCard.tsx
    WeatherIcon.tsx
    weather.css
    index.ts

It had a package.json.

It had tests.

It even had a pleasantly small public export.

export { WeatherCard } from './WeatherCard';

Then I looked at the imports.

import { useSelector } from 'react-redux';
import { useTranslation } from '../../i18n';
import { useExperiment } from '../../experiments';
import { useAnalytics } from '../../analytics';
import { useRouter } from '../../router';
import { selectSavedLocations } from '../../store';

The package was physically separate and architecturally attached to almost everything around it.

An import is at least honest about this. Package managers can see it. Build tools can see it. Static analysis can complain about it.

Other dependencies are more discreet.

Our component appeared to have a wonderfully small API:

<WeatherCard item={item} />

That was not really its API.

Its inputs looked more like this:

WeatherCard
    │
    ├── item
    ├── Redux store
    ├── router
    ├── translations
    ├── experiments
    ├── analytics
    ├── application type
    └── device configuration

Only one happened to be written in the function signature.

The others arrived through the atmosphere.

This wasn't an argument against Redux or React Context. Both had been useful. The problem appeared only when we tried to establish a boundary.

Global context doesn't remove dependencies.

It makes some of them easier not to see.

An adapter helped

We introduced an adapter.

function WeatherCardAdapter({ rawItem }: Props) {
  const savedLocations = useSelector(selectSavedLocations);
  const t = useTranslation();
  const router = useRouter();
  const experiment = useExperiment('compact-weather-card');
  const analytics = useAnalytics();

  return (
    <WeatherCard
      rawItem={rawItem}
      savedLocations={savedLocations}
      translate={t}
      compact={experiment.enabled}
      onOpenLocation={router.open}
      onEvent={analytics.send}
    />
  );
}

WeatherCard no longer needed to know where these things came from.

The application-facing dependencies accumulated in the adapter instead.

Application
     │
     ▼
WeatherCardAdapter
     │
     ▼
 WeatherCard

There was an immediate problem.

The component interface became ugly.

We fixed the ugly interface

The real component looked closer to this:

<WeatherCard
  rawItem={rawItem}
  savedLocations={savedLocations}
  translate={translate}
  compact={compact}
  onOpenLocation={onOpenLocation}
  onSaveLocation={onSaveLocation}
  onEvent={onEvent}
  isMobile={isMobile}
  pageType={pageType}
/>

Nine props.

We had taken a component with a neat API and turned it into this.

So we did what seemed sensible.

We introduced WeatherCardContext.

interface WeatherCardContext {
  savedLocations: SavedLocation[];
  translate: Translate;
  compact: boolean;
  openLocation: (id: string) => void;
  saveLocation: (id: string) => void;
  sendEvent: (event: WeatherEvent) => void;
  isMobile: boolean;
  pageType: PageType;
}

The component became tidy again.

<WeatherCard
  rawItem={rawItem}
  context={weatherCardContext}
/>

Much better.

For perhaps a week, I thought we had found the right abstraction.

Then another card needed the same translation function.

And the same navigation behaviour.

But not the same analytics.

A third card needed analytics and device information, but had a different interpretation of pageType.

We started constructing context objects.

Then helpers for constructing context objects.

Then a common context from which card-specific contexts could be constructed.

The dependency graph we had been trying to remove was slowly reappearing inside one object.

Worse, code review had become harder.

This:

<WeatherCard context={context} />

said almost nothing about what changing WeatherCard might affect.

We had successfully shortened the interface.

We had also hidden it again.

So we deleted WeatherCardContext and put the ugly props back.

That felt like going backwards.

It wasn't.

The long interface was information.

Some props belonged to the weather card:

rawItem
savedLocations

Some were capabilities supplied by the application:

onOpenLocation
onSaveLocation
onEvent

Some described presentation:

compact

And some looked suspicious:

isMobile
pageType

Why did a weather card need to know that it was on mobile?

The answer was in the rendering code.

if (isMobile) {
  return <CompactTemperature temperature={temperature} />;
}

return <TemperatureWithDetails temperature={temperature} />;

It didn't care about mobile.

It cared about presentation.

So:

isMobile={isMobile}

became:

displayMode="compact"

A tiny change, but a useful one.

isMobile described the world outside the card.

displayMode described the card.

The ugly interface had shown us where to look.

Sometimes an ugly interface is an X-ray.

Then we found the state

Saving a location changed global Redux state.

That seemed reasonable. The application needed to know which locations were saved.

The card also stored whether its save animation was running in Redux.

That seemed less reasonable.

Then we found whether the tooltip had been dismissed.

Also Redux.

Whether the expanded forecast was open.

Redux.

Whether the user was currently hovering over the card.

Thankfully, not Redux.

We had apparently drawn a boundary somewhere. We just couldn't explain why it was there.

Asking whether something belonged in Redux wasn't getting us very far, so we changed the question.

Who owns this state?

The expanded forecast existed only while the card existed.

The save animation existed because the card was displaying it.

The tooltip belonged to that interaction.

They moved inside.

function useWeatherCard() {
  const [expanded, setExpanded] = useState(false);
  const [saving, setSaving] = useState(false);
  const [tooltipVisible, setTooltipVisible] = useState(true);

  // ...
}

Saved locations were different.

Another card could save the same location. Navigation could show the number of saved locations. The information survived the lifetime of any individual WeatherCard.

That stayed outside.

This gave us a rule we could actually use:

If state needs to be observed outside a boundary, either the observer belongs inside the boundary or the state doesn't.

It was a more useful question than choosing between Redux and useState.

The application knew what weather looked like

Data was next.

The API returned something like:

{
  "type": "weather",
  "id": "london",
  "temperature": 17,
  "units": "celsius",
  "forecast": [],
  "alerts": [],
  "provider": {}
}

The application parsed this response.

The feed extracted fields.

The card received the resulting object.

This had always worked.

It also meant that changing the server representation of a weather card could require changes in code that otherwise had nothing to do with weather.

We tried making the data opaque to the application.

The feed needed surprisingly little:

interface FeedItem {
  id: string;
  type: string;
  rawItem: unknown;
}

The rest could cross the boundary untouched.

Server
   │
   ▼
Feed
   │
   │ rawItem
   ▼
WeatherCard
   │
   ▼
Weather parser
   │
   ▼
Weather model

The parser moved into the package.

const WeatherItem = object({
  id: string,
  temperature: number,
  units: oneOf('celsius', 'fahrenheit'),
  forecast: array(Forecast),
});

This was one of the quieter changes in the migration.

It removed no visible feature. It probably made no benchmark move.

What changed was who had to understand the data.

The feed stopped understanding weather.

That was useful.

A string that wasn't a string

Packages gave us another tool: compilation boundaries.

Suppose some application code accepted a location identifier.

type LocationId = string;

function openWeather(locationId: LocationId) {
  // ...
}

Later the contract changed. openWeather could no longer accept any location identifier. It required the canonical location selected after resolving aliases.

Unfortunately:

LocationId = string
CanonicalLocationId = string

From TypeScript's point of view, nothing had happened.

The compiler was correct.

It was proving the wrong thing.

We wanted consumers to acknowledge the semantic change, so we gave the new identifier a distinct type.

type CanonicalLocationId = string & {
  readonly __brand: 'CanonicalLocationId';
};

Suddenly the build failed in several places.

Good.

Each error represented a consumer whose assumption needed to be reconsidered.

The branded type wasn't particularly sophisticated. What interested me was what the failure meant.

We had turned part of an architectural migration into something the compiler could inspect.

A boundary described in documentation is an intention. A boundary that can fail the build has teeth.

One experiment, three teams

The compact layout had started as a single experiment.

compact-feed-cards
      │
      ├── WeatherCard
      ├── NewsCard
      └── SportsCard

This had seemed like sensible reuse.

The three cards were testing roughly the same visual idea, so one flag was simpler than three.

Then the weather implementation was ready.

The news implementation was nearly ready.

Sports had discovered an interaction problem and needed another week.

Nothing was broken. There was no technical incident. We simply couldn't finish the weather experiment without deciding what to do about sports.

The options were not attractive.

We could wait.

We could add special-case logic around the shared experiment.

We could coordinate a partial rollout between teams.

Or we could admit that the experiment wasn't actually one thing.

We split it:

compact-weather-card
compact-news-card
compact-sports-card

Three flags where there had been one.

Less DRY.

The weather team shipped.

A few days later news shipped.

Sports changed its implementation and shipped after that.

The interesting dependency had never appeared in the module graph. There was no import from weather to sports. There was no shared component.

They shared a release decision.

That was enough to couple them.

After that I became less interested in counting dependencies and more interested in looking for things that had to change together.

Sometimes that was an import.

Sometimes state.

Sometimes a server format.

Sometimes an experiment.

Sometimes a release process.

They looked different in code.

Operationally, they behaved rather similarly.

The architecture got worse

After the first few migrations, our architecture diagram looked worse than when we started.

Imagine a feed with sixty card types.

We had migrated four.

It now looked roughly like this:

Feed
 ├── LegacyNewsCard ───── Redux ── Context ── ...
 ├── LegacySportsCard ─── Redux ── Context ── ...
 ├── LegacyFinanceCard ── Redux ── Context ── ...
 │
 ├── WeatherCardAdapter
 │       │
 │       ▼
 │   WeatherCard
 │
 ├── TravelCardAdapter
 │       │
 │       ▼
 │   TravelCard
 │
 └── ...54 more

Before the migration there had been one architecture.

Now there were two.

We had more adapters, more package boundaries and more concepts to explain to somebody joining the project.

Meanwhile product development continued.

A legacy card would gain a feature while we were extracting another card. Occasionally somebody would add a new global dependency to a component we intended to migrate next.

For a while this felt like evidence that the migration was too slow.

The tempting response was to accelerate it.

Take ten cards.

Stop touching them for product work.

Move them together.

Clean up the intermediate abstractions afterwards.

On paper this was much more efficient.

It also meant that instead of learning whether the boundary worked from one card, we would learn after ten.

So we kept the migration deliberately uneven.

Four cards became six.

Six became nine.

The remaining cards stayed exactly as awkward as they had been before.

This had an unexpected benefit.

The new architecture had to earn its keep while surrounded by the old one.

If extracting WeatherCard required changing all sixty cards, the design was not useful yet.

If the new package required every feed to adopt a new state model simultaneously, the design was not useful yet.

If a parser could move only after the entire API response was redesigned, it was not useful yet.

The constraints of incremental migration rejected quite a few elegant ideas for us.

I am grateful for that now.

An architecture that works only after the migration is finished has a bootstrapping problem.

For a large, continuously changing system, the intermediate states are not a brief inconvenience between two architectures.

They are where you live.

Our migration would take months.

For most of that time, the transitional architecture was the architecture.

That changed the question we asked about new abstractions.

Not:

Does this get us to the cleanest final design?

More often:

Does this make the next card cheaper to change without making the remaining fifty-one harder to live with?

That produced smaller steps.

It also meant we could stop.

Had priorities changed after twelve cards, we would still have had twelve cards with clearer ownership and forty-eight functioning legacy cards.

There would have been no half-built platform waiting for the rest of the application to catch up.

The destination still mattered.

The ability to stop on the way mattered more than I had expected.

The card became boring

Eventually WeatherCard became surprisingly uninteresting.

It received data.

It parsed the data it owned.

It managed its local interactions.

It asked the application to perform things outside its boundary.

<WeatherCard
  rawItem={rawItem}
  displayMode="compact"
  savedLocations={savedLocations}
  onSaveLocation={saveLocation}
  onOpenLocation={openLocation}
  onEvent={sendWeatherEvent}
/>

It didn't know which state library the application used.

It didn't know which router it used.

It didn't know where experiments came from.

It didn't know whether it was rendered in the main application, Storybook or a test harness.

The adapter knew considerably more.

                   Application
                        │
       ┌────────────────┼────────────────┐
       │                │                │
     Router          State          Experiments
       │                │                │
       └────────────────┼────────────────┘
                        ▼
                WeatherCardAdapter
                        │
                        ▼
                   WeatherCard

We had not eliminated the complexity.

We had changed where knowledge was allowed to accumulate.

I think that distinction matters.

The files were the easy part

Looking back, moving WeatherCard into packages/weather-card was one of the least interesting things we did.

The package became useful only as other changes accumulated around it.

Dependencies that had been ambient became explicit.

State acquired owners.

Data was interpreted closer to the code that understood it.

Some semantic contracts became compilation failures.

Release decisions that looked shared turned out not to be.

And the old application remained operational while all of this happened.

I started with a fairly mechanical picture of modularity:

one large thing
      ↓
several smaller things

That picture isn't wrong.

It just doesn't say very much.

The question I find more useful now is:

If this part of the system changes, how much of the rest of the system needs to know?

For WeatherCard, the answer gradually became: less.

Not nothing.

Just less.

That turned out to be enough.

Teaching an AI to Review Frontend Code

Sergey — Sat, 29 Aug 2026 09:13:56 +0000

I have been experimenting with AI-assisted code review for a while.

The first attempts were not particularly useful.

They were not wrong, exactly. The reviews found things: a questionable name here, a function that might be extracted there, an opportunity for useMemo, perhaps an any that could be made more precise.

There was plenty of activity.

There was rather less reviewing.

What bothered me was the lack of proportion. A naming suggestion and a possible race condition could appear next to each other as though they deserved roughly the same amount of attention.

They do not.

That observation led to a small experiment.

Instead of asking an AI to find problems in code, I tried to describe how I wanted a review to proceed.

I ended up writing a code-review skill.

This is a story about what went into it, what changed along the way, and what the exercise taught me about code review itself.

The first version was a checklist

This seemed sensible.

Frontend code has plenty of things worth checking:

TypeScript types
React effects
accessibility
error handling
performance
tests
state management
API contracts
naming
duplication

So I wrote them down.

The result was comprehensive.

It was also not very good.

Given enough checklist items, an AI can nearly always find something to say. The difficulty is that software engineering is rarely about finding the largest number of technically defensible observations.

It is about deciding what matters.

Consider a small change:

useEffect(() => {
  fetchUser(userId).then(setUser)
}, [userId])

There are several things one might say about these four lines.

But the interesting question, for me, is what happens when userId changes before the first request completes.

Request A starts for user 10.

The component changes.

Request B starts for user 11.

B completes first.

Then A completes.

The code is still short. The types may still be correct. The linter may be entirely content.

The UI may now be showing the wrong user.

That was the first useful shift in the skill:

Review the behaviour around the code, not merely the code in the diff.

A review needs an idea of what matters

Once I had noticed this, the checklist began to look like the wrong abstraction.

The skill needed priorities.

I eventually wrote this near the beginning:

Review code as a production risk audit, not a style pass.

That sentence became surprisingly useful.

It gives the review a direction.

A resource leak matters before a naming preference.

A broken permission boundary matters before duplicated code.

Losing a user's changes matters before whether a helper should be extracted.

An out-of-order response matters before an unnecessary allocation.

This does not make maintainability, performance or style irrelevant.

It simply gives them somewhere to stand.

The review starts with things that can break user trust or cause the running system to diverge from its intended behaviour: lifecycle leaks, asynchronous races, state integrity, permissions, persistence and other high-impact paths.

That ordering made the reviews noticeably quieter.

I considered that progress.

Contract before implementation

The next problem was more subtle.

How can a reviewer decide whether code is correct without first deciding what the code is supposed to do?

This sounds obvious when written down.

In practice, I had been skipping it.

So the first review gate became Contract.

The contract is not necessarily written in one place. It may be distributed across the pull-request description, types, tests, public exports, documentation, neighbouring code and the vocabulary of the project.

Even the word refactor contains a contract.

If I say:

This is only a refactor.

I am making a fairly strong claim:

Observable behaviour has not changed.

Similarly:

This improves performance.

contains another claim:

There is a measurable improvement, and the relevant behaviour has been preserved.

Once I started treating those statements as claims rather than descriptions, the review changed.

Instead of asking:

Is this implementation good?

the more useful question became:

What has this change promised, and what would convince me that the promise holds?

I like this framing because it removes quite a lot of personal taste from the conversation.

The diff is smaller than the change

Another thing I kept running into was the boundary of the pull request.

A prop changes in one component, but the behavioural change belongs to every consumer.

A query key changes in one hook, but the consequence belongs to the cache.

An API field changes in one type, but the actual surface may run through validation, mapping, persistence and several pieces of UI.

A new option is particularly interesting.

Adding it is easy.

Working out what every existing consumer does when it encounters that option is often where the real review begins.

So I added another gate: Impacted surface.

The instruction is essentially to follow changed values and behaviour through the system: props, state, API payloads, events, permissions, loading states, errors, cache keys, exports and side effects.

This is expensive compared with commenting on the changed lines.

It is also much closer to what I find myself doing when reviewing an important change manually.

The diff tells me where the edit happened.

It does not necessarily tell me where the change happened.

Time deserves its own review

Frontend software has another awkward property: much of its behaviour exists across time.

A component renders, mounts, updates and unmounts.

A request starts, gets cancelled, retries, resolves or becomes stale.

A form is initialised, edited, validated, submitted, rejected, reset and perhaps abandoned halfway through.

Looking at one moment in these processes hides a surprising number of bugs.

So the skill began to describe lifecycles explicitly:

Component
render → hydrate → mount → update → suspend → unmount

Async work
start → cancel → retry → resolve/reject → stale result → cleanup

Form
initialise → edit → dirty → validate → submit → error/success → reset

Data
fetch → normalise → validate → cache → derive → render → mutate → invalidate

These are not meant as universal state machines.

They are prompts for attention.

When an effect changes, I want the review to follow it through unmount.

When a form changes, I want to know what happens after the server rejects the submission.

When caching changes, I want to know how stale data eventually becomes fresh again.

This turned out to be one of the more useful additions.

Evidence changed the tone of the review

There was still a problem.

AI is rather good at producing plausible concerns.

This could cause unnecessary renders.

There may be a race condition here.

Consider memoising this value.

Each sentence sounds reasonable.

None of them is necessarily true.

I did not want to solve this by making the model more confident.

Quite the opposite.

I wanted uncertainty to become part of the output.

That led to the Evidence gate.

A correctness change should have a regression test, or a reason why one is impractical.

A behaviour-preserving refactor should have evidence that the relevant old behaviour still exists.

A performance improvement should have a measurement.

And when the available context cannot establish something, the review should say so.

For example:

I cannot establish whether this request is cancelled when the component unmounts. The caller lifecycle or a focused cancellation test would close that gap.

I find that much more useful than:

Potential memory leak.

The former tells me what is known, what is not known and how to reduce the uncertainty.

The latter mostly tells me that a memory leak is imaginable.

Perhaps unsurprisingly, this also made the reviews less argumentative.

Evidence is easier to discuss than confidence.

Refactoring needed a different rule

At some point I tried using the same skill for refactoring.

That exposed another distinction.

When I ask for a review, I want the system to challenge behaviour.

When I ask for a refactor, I usually want the opposite: improve the implementation while being extremely conservative about behaviour.

So refactoring acquired a rather strict constraint:

Zero behaviour change unless explicitly approved.

Public APIs, execution order, side effects, error semantics and observable identity all matter here.

Even a cleaner implementation is not an improvement if it quietly changes one of them.

This also changed how I think about abstraction.

Two pieces of code looking similar is not sufficient reason to combine them.

I now prefer to ask whether they share a contract and the same edge cases.

If they do, perhaps there is an abstraction.

If they merely happen to contain similar lines today, duplication may be cheaper.

What the skill looks like now

After several iterations, the structure became smaller than the original checklist.

Conceptually, it is roughly:

Contract
   ↓
Impacted surface
   ↓
Failure & divergence
   ↓
Evidence
   ↓
Lower-priority quality

There are specialised expansions underneath it for effects, forms, caches, permissions, dates, APIs, generated files, dependencies and other areas.

But those are secondary.

The sequence is the important part.

It tells the reviewer where to spend attention before telling it what details it might notice.

That distinction took me longer to see than I expected.

I thought I was writing instructions for an AI

The slightly unexpected part of this experiment is that I am no longer sure the skill is primarily about AI.

Writing it forced me to make some of my own review habits explicit.

Why do I sometimes open five files before commenting on one changed line?

Because I am tracing the impacted surface.

Why does an innocent-looking useEffect make me uncomfortable?

Usually because I have not yet accounted for its lifecycle.

Why am I reluctant to approve a refactor even though the new code is obviously cleaner?

Because cleaner is visible in the diff.

Behaviour preserving needs evidence.

Why do some reviews with fifteen comments feel less useful than reviews with one?

Because the number of observations is not the same thing as the amount of uncertainty removed.

I started with a fairly mechanical question:

How should an AI review frontend code?

I ended up with a different one:

What do I actually do when I believe I am reviewing code well?

The skill is my current answer.

I expect it to change.

That may be the most useful property it has.