piro

Posted on Aug 23

Someone forked my React component instead of opening an issue

#react #javascript #typescript #opensource

I maintain a small comic and manga viewer component for React called react-comic-viewer.

The other day I was poking around npm and noticed something odd — there were three other packages with basically my package's name, published by other people.

All three were forks of mine. Same description, same repository URL pointing back at my repo.

None of the three authors had ever opened an issue or a pull request on my side.

What the fork changed

The oldest fork was made about three months after I first published, and it kept going for almost a year. Its version number ran ahead of mine at the time — I was on 0.3.5 while the fork was on 0.6.3.

So I read the diff. Honestly, it was more useful than any issue would have been.

The commit messages alone told the whole story:

remove sass
fix: support className props
use Hotkeys
and a new example file called controlled.tsx

The sass one I'd already fixed. The className one I'd fixed too, about a year later.

The controlled.tsx one I had never fixed. Not in four years.

The part I never fixed

Here's what their example looked like:

<ComicViewer
  currentPage={currentPage}
  isExpansion={false}
  onTryMoveNextPage={(nextPage) => { /* ... */ }}
  onChangedCurrentPage={(page) => setCurrentPage(page)}
  pages={pages}
/>

And here's what my component actually accepted:

<ComicViewer
  initialCurrentPage={0}
  initialIsExpansion={false}
  onChangeCurrentPage={(page) => { /* ... */ }}
  pages={pages}
/>

The initial prefix is the whole problem.

My component would take a starting page from you, and then never let you touch it again. It owned that state for the rest of its life.

That's fine for a demo. It's pretty bad for anything real.

You can't jump to a page from a table of contents. Syncing the current page with the URL doesn't work either. And if a chapter needs to be purchased first, there's no way to step in and stop the move.

Every one of those needs the parent to be in charge, and the parent never was.

Making it controllable

This is the standard controlled/uncontrolled pattern, and there's nothing clever about it — but it took me an embarrassingly long time to get around to it, so here's the shape I ended up with:

function useControllableState<T>(
  controlledValue: T | undefined,
  defaultValue: T,
  onChange?: (value: T) => void,
) {
  const [uncontrolledValue, setUncontrolledValue] = useState(defaultValue);
  const isControlled = controlledValue !== undefined;
  const value = isControlled ? controlledValue : uncontrolledValue;

  const valueRef = useRef(value);
  valueRef.current = value;
  const onChangeRef = useRef(onChange);
  onChangeRef.current = onChange;

  const setValue = useCallback(
    (next: T | ((prev: T) => T)) => {
      const resolved =
        typeof next === "function"
          ? (next as (prev: T) => T)(valueRef.current)
          : next;
      if (resolved === valueRef.current) return;
      if (!isControlled) setUncontrolledValue(resolved);
      onChangeRef.current?.(resolved);
    },
    [isControlled],
  );

  return [value, setValue] as const;
}

Two things worth pointing out.

The refs are there so the setter stays stable even when you pass an inline arrow function as onChange. Without them, every render hands you a new setter, and everything downstream re-renders with it.

And the onChange call lives inside the setter, not in a useEffect watching the value. That matters — in controlled mode the value doesn't change locally at all, so an effect watching it would simply never fire, and the parent would hear nothing.

Two spots that got awkward

Two places inside the component wrote to that state on their own.

Going fullscreen forces the expanded view, and restores the old value on exit. And switching to a two-page spread rounds the current page down to an even number.

In controlled mode, neither of those can just happen anymore. The parent owns the value.

I settled on the boring answer: the component asks, through onChange, and if the parent ignores it, nothing moves. That's how a controlled input behaves too — type into one without wiring up onChange and the text just doesn't appear.

Slightly surprising the first time, but it's the honest behavior, and it's what everyone else already does.

What shipped

1.1.0 adds currentPage and isExpansion as optional props. Pass either one and you own it; leave it out and the old behavior is untouched. Each is independent, so you can control the page and let the component keep the expansion state.

It also adds onTryMoveNextPage and onTryMovePrevPage, which fire before the page changes, and lets an entry of pages be a function receiving the class name the viewer would have applied to its own <img> — so you can bring your own element for lazy loading.

There's a demo at react-comic-viewer.kkweb.io if you want to poke at it.

The thing I keep coming back to isn't the code, though.

Somebody hit a wall in my library, solved it properly, published the fix under their own name, and I found out four years later by accident.

That's not on them. Forking is cheaper than filing an issue and waiting, and I'd probably do the same.

I just wish I'd looked sooner. Four years is a long time to leave something sitting there.

DEV Community