pulsar icon indicating copy to clipboard operation
pulsar copied to clipboard

Refactor `StateStore` to remove dependency on the `atom` global

Open savetheclocktower opened this issue 9 months ago • 0 comments

I had occasion to visit src/state-store.js and thought I'd refactor it a bit.

I know we have this annoying problem where we want to rely on the atom global, but we end up acting before it's defined. This is vexing to StateStore, since it wants to read from config to decide which kind of adapter to use (IndexedDB or SQL). In this case, we can pass the config instance into the constructor, dependency-injection–style. By the time we actually need to read the config, it's been initialized and is ready to read values.

The SQL adapter also needs to know where to store its DB, so it reads atom.getConfigDirPath(). This value can't be introspected from atom.config, and it isn't even ready by the time we create StateStore; it gets initialized later in the environment setup process. The fix is similar to the fix for a lot of similar objects created during bootstrapping: a second “initialization” phase where the later properties can get set. This is still early enough to be set before the first time we need to actually use StateStore, so it all works out.

This allows us to simplify the implementation of src/state-store.js and make some more methods synchronous. (They still go async later, but the code is easier to read this way.) We also no longer need to do the awkward thing where we check for the atom global every 50ms.

All the relevant tests seem to pass locally, but let's see what CI says.

savetheclocktower avatar Mar 25 '25 07:03 savetheclocktower