Compose some site scripts to reduce complexity of package.json

[kachkaev]

This PR is a quick experiment based off discussions in #157. It unifies CLI usage of TS and reduces the complexity of package.json → scripts.

Script composition will get lighter when we unlock top-level await (it has no timeout).

Now

export default script();

await (
  await import("./another-script")
).default;

With TLA (when we unlock it)

await script();

await import("./another-script");

Suggested ‘Rules of Scripts’

• All scripts are located in the /scripts/ folder of the monorepo root or of a workspace.

• To call a script: yarn exe scripts/abc/def.ts, where exe is a shortcut to ts-node or some other TS runtime. Bash autocompletion adds file extension, so let’s not omit it in docs etc. for consistency.

• console.log is allowed in **/scripts/**, unlike in the rest of the codebase.

• Each script file contains

  const script = async () => {
    // ...
  }
  
  export default script()

  This makes scripts composable. It is recommended to not include script logic outside script function as it makes it harder to follow.

• CLI arguments are not allowed, we are only using env vars. Validation can be done via envalid like in Add envalid to improve script feedback when env vars are missing #192

• Scripts are not included into published NPM packages. If we want to use CLI in packages, we should write single-entry-point commands instead (e.g. with commander). Commands are harder to maintain, but they can be called as npx script-name arg1 arg2 --foo=bar. We can share logic between scripts and commands can calling the same shared functions from them.

• Scripts don’t contain prompts, i.e. don’t interrupt for user input. If sufficient info was not provided via env vars, a script exits with error. This constraint improves composability and makes scripts immediately available in CI.

• If a workspace contains a containerizable app, scripts can (and should) be included into an image.

• Ideally we want some bold console.log at the beginning of each script and some sort of ‘Done.’ in the end. This will improve stdout of composed flows.

Caveat

A sub-script can be invoked only once because

await import("./a");
await import("./a");

is the same as

await import("./a");

Maybe it’s a benefit actually. If we want to call some functionality multiple times, we should make it a ‘normal’ export of some shared *.ts file.

[kachkaev]

We need to switch to "type": "module" in package.jsons to upgrade to chalk 5 and execa 6 (they are ESM-only). We can do this when vercel/next.js#33637 is merged and we’ve upgraded Next.js to latest.

[thehabbos007]

This might not be an actual problem, but it seems that execa buffers stdout by default seen here. I am wondering if we are going to hit that limit realistically? Probably not, but worth keeping in mind.

[kachkaev]

Good point! It’s 100MB by default though, so I hope it will be enough 😅

Alternatively, we can launch Next.js server here directly, i.e. in the same process. That would mean that we switched to a custom server though – not sure if it’d be a good thing to do.

[thehabbos007]

Wonder what will happen at the buffer limit. I assume the buffer simply wraps around and still outputs to stdout, so perhaps it's a non-issue

[benwerner01]

To help enforce constancy between the logging messages's formatting using chalk and use of emojis, what if we create a wrapper function like this:

const runScript = async (
  script: () => Promise<void>,
  config: { startMsg: string; successMsg: string; errorMsg: string; }
) => {
  try {
    console.log(chalk.bold(`${config.startMsg}...`));
    await script();
    console.log(`✅ ${config.successMsg}`);
  } catch (error) {
    console.log(`❌ ${config.errorMsg}: `, error)
  }
}

(this could also be simplified to just accepting a single scriptName: string argument, and then logging Running script ${scriptName}..., ✅ Completed script "${scriptName}", etc)

This would let developers focus on writing the functionality of the scripts:

...
const script = async () => {
  const sitemap = generateSiteMap();

  writeFileSync(
    path.join(process.cwd(), `site-map.json`),
    JSON.stringify(sitemap, null, "\t"),
  );
};

export default runScript(script, {
  startMsg: "Generating sitemap",
  successMsg: "Site map generated",
  errorMsg: "Could not generate sitemap"
});

[kachkaev]

Thanks Ben! I tried creating similar abstractions for scripts in some side projects, but found them more costly than beneficial in the end. Imagine we have a script that generates a file and let’s say we don’t want to override the result unless FORCE=true is provided. So our messages will look something like:

[bold] Generating our file...
[magenta] File /x/y/z generated.

[bold] Generating our file...
[gray] File /x/y/z already exists, use FORCE=true to overwrite it.

[bold] Generating our file (with FORCE=true)...
[magenta] File /x/y/z regenerated.

As you can see, startMsg and successMsg vary. File path (/x/y/z) can vary too if it’s in some configurable DATA_DIR_PATH directory. This is just one possible scenario of what a script might want to do – the reality will be full of diversity.

My current thinking leans towards using standard logging functions plus chalk. This is simple, flexible and solid enough. There is not much added repetition and the cost of forgetting to log something is quite small. On the other hand, we don’t need to learn how to use a new wrapper abstraction or add features to it.

Here are some example scripts I wrote for a side project: https://github.com/kachkaev/tooling-for-how-old-is-this-house/tree/4e1c420a88fb6c3d75fef17729eeed9c720fadc3/src/scripts. There are tens of them, all ‘wrapper-free’ (and with top-level await already 🤓).

The only abstraction I find useful these days is ScriptError. It is 100% like a normal Error, but if a script throws it all the way up, Node does not print the call stack. It’s an alternative to writing this, but with a more familiar syntax.

console.log(chalk.red("Some friendly message explaining why script failed"));
process.exit(42);

// ↓

throw new ScriptError("Some friendly message explaining why script failed");

WDYT?

[benwerner01]

Thanks Alex, that makes sense to me - you're right we shouldn't sacrifice flexibility when abstracting how we log messages in the scripts.

One way to avoid developers from having to worry about usage of chalk and emojis could be an abstraction such as ScriptError as you suggest, but I don't feel strongly either-way about this. I'm happy for this to be left as is.

[kachkaev]

Some helper logging lib would be useful indeed! I did a quick research a few months ago, but could not find anything compelling. We don’t expect to handle stdin in scripts, so we don’t need anything interactive like prompts.

If we introduce some third-party util, what we’ll be using at most is some combination of console.log + chalk. I don’t know how much that will help, but I’m happy try out a new lib if anyone has suggestions!

[thehabbos007]

Nice to have the scripts be orders in code instead of package.json.
I hit a snag with the seed-db.ts script. It seems that since we are using this connection helper

blockprotocol/site/src/lib/mongodb.ts

Lines 7 to 13 in f8de754

	let cachedClient: MongoClient | null = null;
	let cachedDb: Db | null = null;
	export const connectToDatabase = async () => {
	if (cachedClient && cachedDb) {
	return { client: cachedClient, db: cachedDb };
	}

there is an implicit cached client which is invalidated when .close() is called on the client. This means composing scripts will lead to errors like:

yarn run v1.22.17
$ ts-node --require dotenv-flow/config --transpile-only --transpiler=ts-node/transpilers/swc-experimental site/scripts/seed-db.ts
Seeding DB...
✅ DB seeded
Creating DB indexes...
❌ Error trying to create index, MongoNotConnectedError: MongoClient must be connected to perform this operation
❌ Error trying to create index, MongoNotConnectedError: MongoClient must be connected to perform this operation
❌ Error trying to create index, MongoNotConnectedError: MongoClient must be connected to perform this operation
❌ Error trying to create index, MongoNotConnectedError: MongoClient must be connected to perform this operation
✅ MongoDB indexes created
Done in 1.07s.

where one script succeeds and one fails in grouped scripts.

Perhaps we can introduce a noCache argument or invalidate the cached client somehow.

[kachkaev]

Great spot @thehabbos007! I hope I have fixed this in a3c3a86.

I also tried removing await client.close() from both scripts – but this did not work. Scripts were not exiting, probably because of some inner unresolved promises or open sockets in the client instance.

[thehabbos007]

Great, thank you! All commands work well, and i see the requested changes above have been considered