Stencil Store Challenge
📖 tl;dr: With some minor cosmetic changes we can make code smaller and easier to read.
When Manu from the Stencil team pinged me on Twitter regarding a code golfing challenge, I just couldn't resist! So I thought this could be a good exercise to apply techniques we use while working on Preact to a codebase that's foreign to me.
Getting ready
After the usual git clone
and npm install
we're good to go. Before we dive in immediately, we should get ourself familar with the overall goal the code is trying to clear. From the looks of it we're dealing with an extension to the Stencil framework which attempts to make state management easier. For that they rely on smaller stores where each property can be observed. This library holds the cold for that.s
Overall I'm getting a Svelte vibe here as they have a similar concept in their frameworks. I expect more frameworks follow in the coming months, but only time will tell for sure. The codebase is authored in TypeScript which is a godsend whenever you have to get familiar with foreign code!
Readability
The first things I usually do is to remove anything that's just noise. In particular with TypeScript, developers tend to write overly verbose type declarations, even though TypeScript can infer most of the types automatically. One of such instances can be seen in the implementation for a Map
-like data structure, that's observable:
const use = (...subscriptions: StoreSubscriptionObject<T>[]): void =>
subscriptions.forEach((subscription: StoreSubscriptionObject<T>): void => {
//...
});
};
Luckily for us TypeScript is smart enough to unwrap boxed types like arrays, whenever we iterate over it. We can just drop the verbose type name here. The return type is also not needed as the whole function doesn't make use of the return
keyword. Let's drop that too! Since the whole code in this repo is pretty small we don't have to go all enterprise with our naming convention and can shorten StoreSubscriptionObject
to just Subscription
.
const use = (...subscriptions: Subscription<T>[]) =>
subscriptions.forEach(subscription => {
//...
});
};
Much better! Readabilty is the very foundation to write shorter code. It sets the stage in writing style in which any new code usualy adheres to. This process rarely happens concious, but rather in our unconciesness, because for us humans it's natural to want to fit into the group. If the writing style favors verbosity it makes it harder (at least for me) to quickly scan code and immediately know what it does. Information densitiy is key here! But don't overdo it!
Note: There are many whacky eslint presets for TypeScript out there that force you to manually declare every single type. This is pretty bonkers and completely ignores a huge strength of a mature type system.
Let the golf begin!
Looking at the whole function body, we notice a list of if
-statements, that just beg to be optimized.
const use = (...subscriptions: Subscription<T>[]) =>
subscriptions.forEach(subscription => {
if (subscription.set) {
on("set", subscription.set);
}
if (subscription.get) {
on("get", subscription.get);
}
if (subscription.reset) {
on("reset", subscription.reset);
}
});
If we check the type declaration of Subscription
(formely StoreSubscriptionObject
), we can see that this function checks for all possible properties and adds a listener to them. Armed with that knowledge we can rewrite the code to iterate over all properties and assign a listener automatically:
const use = (...subscriptions: Subscription<T>[]) =>
subscriptions.forEach(subscription => {
Object.keys(subscription).forEach(key => on(key, subscription[key]));
});
Although the rewritten function is much shorter I'm not really satisfied with it, as it introduces an iteration in a section which is potentially executed quite a lot. Maybe we can change the way subscriptions are handled itself. The current way relies on each event calling a separate function: One for "set"
, one for "get"
and another one for "reset"
. When it comes to code-golfing function declarations tend to be on the expensive side, same as strings. So maybe, if we manage to it right, we can kill two birds with one stone.
We're dealing only with a single Subscription type here which is closely tied to a Map
.
export interface Subscription<StoreType> {
get?<KeyFromStoreType extends keyof StoreType>(key: KeyFromStoreType): void;
set?<KeyFromStoreType extends keyof StoreType>(
key: KeyFromStoreType,
newValue: StoreType[KeyFromStoreType],
oldValue: StoreType[KeyFromStoreType]
): void;
reset?(): void;
}
Even though all properties are declared as optional, they're always used together in the code that's provided. There is not a single place where only one of those is used. So how are subscriptions defined? Looking at the internals of ObservableMap
we see 3 separate arrays for each event. This is a good thing as a shared object structure would likely lead to more allocations.
const setListeners: SetEventHandler<T>[] = [];
const getListeners: GetEventHandler<T>[] = [];
const resetListeners: ResetEventHandler[] = [];
// The `on()` function is used to create subscriptions
const on: OnHandler<T> = (eventName, callback) => {
let listeners: any[];
if (eventName === "set") {
listeners = setListeners;
} else if (eventName === "get") {
listeners = getListeners;
} else if (eventName === "reset") {
listeners = resetListeners;
} else if (Build.isDev) {
throw new Error("Unknown event " + eventName);
}
listeners.push(callback);
};
To set up subscriptions the on(event, callback)
function is used. Looking at the function body we find another long if block that doesn't bring us joy. The number of possible events is finite and limited to 3, so we could leverage a similar trick like the one we applied earlier to get rid of the if
-statement. We can even remove the error message as the browser's native one is pretty similar when dealing with missing object keys.
const handlers: Handlers<T> = {
get: [],
set: [],
reset: [],
};
const on: OnHandler<T> = (eventName, callback) => {
handlers[eventName].push(callback);
};
This brings us much more joy! So far we've only dealt with more "cosmetic" code changes. To be able to really unify the subscription process we have to be a bit more agressive and make changes to the user facing API. This requires more knowledge about how this library is actually used, but I'm not familiar enough with Stencil to make that call. Without that optimzation usually tends to break features.
That's it for now
There are probably more areas we can improve on, but I think I've spent enough time here. I did conciously not go into optimizations which would save a byte here and there, because the library seems to be newer and in heavy development. ASome potential avenues that immediately come to mind:
-
Inline every function that is only used once. This needs to be backed up with numbers, but in general fewer functions compress better with terser+gzip. This is not always the case though, so one should really measure the impact.
-
Refactor
on(event, callback)
toon(callback)
and pass the event type as the first argument. This obviously breaks the existing API and pushes the check which event we're dealing with to the consumer. It would also shift the performance characteristics, so be careful here! On the plus side we could essentially delete a lot of subscription handling logic, so it's definitely a tradeoff to make.
Conclusion
Hope this short adventure helped to portray the thinking process of what it's like to work on Preact. The main takeaways for me from working on Preact are to really understand what the code in front of you is trying to accomplish and to be aware of the context it is used in. Combine that with a laser sharp focus on readabilty and you have the perfect foundation for writing clear and concise code in your heart.