[loevborg]

Can you give an example? Looks fairly decent to me

[q3k]

  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.

[delamon]

What is wrong with peeking at process.env? It is a global map, after all. I assume, of course, that they don't mutate it.

[q3k]

It's implicit state that's also untyped - it's just a String -> String map without any canonical single source of truth about what environment variables are consulted, when, why and in what form.

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).

[lioeters]

> process.env? It is a global map

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.

[withinboredom]

environment variables can change while the process is running and are not memory safe (though I suspect node tries to wrap it with a lock). Meaning if you check a variable at point A, enter a branch and check it again at point B ... it's not guaranteed that they will be the same value. This can cause you to enter "impossible conditions".

[efskap]

Wait, is it expected for them to be able to change? According to this SO answer [0] it's only really possible through GDB or "nasty hacks" as there's no API for it.

[0] https://unix.stackexchange.com/questions/38205/change-enviro...

[withinboredom]

The process itself (including other threads) can call setenv whenever it wants.

[hu3]

For one it's harder to unit test.

[loevborg]

You're right about process.argv - wow, that looks like a maintenance and testability nightmare.

[darkstar_16]

They use claude code to code it. Makes sense

[s3p]

It probably exists only in CLAUDE or AGENTS.md since no humans are working on the code!

[Insensitivity]

the "useCanUseTool.tsx" hook, is definitely something I would hate seeing in any code base I come across.

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

[Overpower0416]

  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;
    }
  }

Why even use else if with return...

[worksonmine]

> Why even use else if with return...

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.

[Overpower0416]

Using guard clauses. Way more readable and easy to work with.

  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;
  }

[kelnos]

I always write code like that. I don't like early returns. This approximates `if` statements being an expression that returns something.

[whilenot-dev]

> This approximates `if` statements being an expression that returns something.

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;
  }

[catlifeonmars]

I’m not strongly opinionated, especially with such a short function, but in general early return makes it so you don’t need to keep the whole function body in your head to understand the logic. Often it saves you having to read the whole function body too.

But you can achieve a similar effect by keeping your functions small, in which case I think both styles are roughly equivalent.

[duckmysick]

I'm not that familiar with TypeScript/JavaScript - what would be a proper way of handling complex logic? Switch statements? Decision tables?

[catlifeonmars]

Here I think the logic is unnecessarily complex. isQuoted is doing work that is implicit in the token.

[luc_]

Fits with the origin story of Claude Code...

[werdnapk]

insert "AI is just if statements" meme

[loevborg]

useCanUseTool.tsx looks special, maybe it'scodegen'ed or copy 'n pasted? `_c` as an import name, no comments, use of promises instead of async function. Or maybe it's just bad vibing...

[Insensitivity]

Maybe, I do suspect _some_ parts are codegen or source map artifacts.

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

[matltc]

Lol even the name is crazy

[wklm]

have a look at src/bootstrap/state.ts :D