1. Randomly peeking at process.argv and process.env all around. Other weird layering violations, too.
2. Tons of repeat code, eg. multiple ad-hoc implementations of hash functions / PRNGs.
3. Almost no high-level comments about structure - I assume all that lives in some CLAUDE.md instead.Such state should be strongly typed, have a canonical source of truth (which can then be also reused to document environment variables that the code supports, and eg. allow reading the same options from configs, flags, etc) and then explicitly passed to the functions that need it, eg. as function arguments or members of an associated instance.
This makes it easier to reason about the code (the caller will know that some module changes its functionality based on some state variable). It also makes it easier to test (both from the mechanical point of view of having to set environment variables which is gnarly, and from the point of view of once again knowing that the code changes its behaviour based on some state/option and both cases should probably be tested).
That's exactly why, access to global mutable state should be limited to as small a surface area as possible, so 99% of code can be locally deterministic and side-effect free, only using values that are passed into it. That makes testing easier too.
[0] https://unix.stackexchange.com/questions/38205/change-enviro...
It's extremely nested, it's basically an if statement soup
`useTypeahead.tsx` is even worse, extremely nested, a ton of "if else" statements, I doubt you'd look at it and think this is sane code
export function extractSearchToken(completionToken: {
token: string;
isQuoted?: boolean;
}): string {
if (completionToken.isQuoted) {
// Remove @" prefix and optional closing "
return completionToken.token.slice(2).replace(/"$/, '');
} else if (completionToken.token.startsWith('@')) {
return completionToken.token.substring(1);
} else {
return completionToken.token;
}
}
What is the problem with that? How would you write that snippet? It is common in the new functional js landscape, even if it is pass-by-ref.
export function extractSearchToken(completionToken: {
token: string;
isQuoted?: boolean;
}): string {
if (completionToken.isQuoted) {
return completionToken.token.slice(2).replace(/"$/, '');
}
if (completionToken.token.startsWith('@')) {
return completionToken.token.substring(1);
}
return completionToken.token;
}Do you care to elaborate? "if (...) return ...;" looks closer to an expression for me:
export function extractSearchToken(completionToken: { token: string; isQuoted?: boolean }): string {
if (completionToken.isQuoted) return completionToken.token.slice(2).replace(/"$/, '');
if (completionToken.token.startsWith('@')) return completionToken.token.substring(1);
return completionToken.token;
}But you can achieve a similar effect by keeping your functions small, in which case I think both styles are roughly equivalent.
But if you take a look at the other file, for example `useTypeahead` you'd see, even if there are a few code-gen / source-map artifacts, you still see the core logic, and behavior, is just a big bowl of soup