Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Compose some site scripts to reduce complexity of package.json #189

Merged
merged 6 commits into from
Feb 8, 2022
Merged

Compose some site scripts to reduce complexity of package.json #189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
91e404a
Compose some site scripts to reduce complexity of `package.json`
kachkaev Feb 3, 2022
0274cb3
Reduce diff
kachkaev Feb 3, 2022
243e95c
Add logging
kachkaev Feb 4, 2022
bfaa398
Add extension
kachkaev Feb 4, 2022
f8de754
Merge branch 'main' into ak/composable-scripts
kachkaev Feb 8, 2022
a3c3a86
Reconnect to MongoDB if client.close() was called previously
kachkaev Feb 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
"@typescript-eslint/eslint-plugin": "4.29.0",
"@typescript-eslint/parser": "4.29.0",
"algoliasearch": "^4.12.0",
"chalk": "^4.1.2",
"check-dependency-version-consistency": "^1.6.0",
"concurrently": "^6.5.1",
"dotenv-flow": "3.2.0",
Expand Down
3 changes: 3 additions & 0 deletions scripts/packages/publish-to-local-registry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import execa from "execa";
import sleep from "sleep-promise";
import path from "path";
import chalk from "chalk";
import { logStepEnd, logStepStart } from "../shared/logging";

// These variables are hardcoded on purpose. We don’t want to publish to a real registry by mistake.
Expand All @@ -26,6 +27,8 @@ const defaultExecaOptions = {
} as const;

const script = async () => {
console.log(chalk.bold("Publishing to local registry..."));

logStepStart("Login into local registry");

if (!npmRegistry.includes("localhost")) {
Expand Down
4 changes: 4 additions & 0 deletions scripts/packages/smoke-test-create-block-app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import { promisify } from "util";
import fs from "fs/promises";
import os from "os";
import untildify from "untildify";
import chalk from "chalk";
import { logStepStart, logStepEnd } from "../shared/logging";

/**
* Calling `execa.kill()` does not terminate processes recursively on Ubuntu.
* Using `killProcessTree` helps avoid hanging processes.
Expand All @@ -34,6 +36,8 @@ const defaultExecaOptions = {
} as const;

const script = async () => {
console.log(chalk.bold("Smoke-testing create-block-app..."));

const tmpNodeCacheDir = await tmp.dir({ unsafeCleanup: true });

const blockName = process.env.BLOCK_NAME ?? "test-block";
Expand Down
6 changes: 3 additions & 3 deletions scripts/shared/logging.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
let currentMarkEndOfPhase: (() => void) | undefined = undefined;
let currentMarkEndOfStep: (() => void) | undefined = undefined;
kachkaev marked this conversation as resolved.
Show resolved Hide resolved

export const logStepStart = (phaseName: string) => {
console.log("");
Expand All @@ -10,7 +10,7 @@ export const logStepStart = (phaseName: string) => {

const startTime = Date.now();

currentMarkEndOfPhase = () => {
currentMarkEndOfStep = () => {
const endTime = Date.now();

const doneMessage = `Done in ${Math.round(
Expand All @@ -26,5 +26,5 @@ export const logStepStart = (phaseName: string) => {
};

export const logStepEnd = () => {
currentMarkEndOfPhase?.();
currentMarkEndOfStep?.();
};
10 changes: 6 additions & 4 deletions site/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@
"private": true,
"license": "MIT",
"scripts": {
"build": "yarn build-blocks && yarn exe scripts/generate-blocks-data && yarn exe scripts/generate-sitemap && yarn exe scripts/create-db-indexes && next build",
"build": "yarn exe scripts/build.ts",
"build-block": "./scripts/build-blocks.sh",
"build-blocks": "find ../hub -type f | xargs ./scripts/build-blocks.sh",
"clean": "rimraf ./.next/",
"dev": "yarn exe scripts/generate-sitemap && yarn exe scripts/generate-blocks-data && yarn exe scripts/create-db-indexes && next dev",
"dev": "yarn exe scripts/dev.ts",
"dev:db": "docker-compose -f docker-compose.dev.yml --env-file .env.local up",
"dev:seed-db": "yarn exe scripts/seed-db && yarn exe scripts/create-db-indexes",
"dev:seed-db": "yarn exe scripts/seed-db.ts",
"exe": "ts-node --require dotenv-flow/config --transpile-only --transpiler=ts-node/transpilers/swc-experimental",
"lint:tsc": "tsc --noEmit",
"prepare": "patch-package && yarn exe scripts/generate-sitemap && yarn exe scripts/generate-blocks-data",
"prepare": "patch-package && yarn exe scripts/prepare.ts",
"start": "next start"
},
"dependencies": {
Expand Down Expand Up @@ -93,6 +93,8 @@
"@types/react-slick": "^0.23.7",
"@types/uuid": "^8.3.4",
"babel-plugin-inline-react-svg": "2.0.1",
"chalk": "^4.1.2",
"execa": "^5.1.1",
Comment on lines +96 to +97
Copy link
Contributor Author

@kachkaev kachkaev Feb 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"rimraf": "^3.0.2",
"ts-node": "^10.4.0",
"typescript": "^4.5.4"
Expand Down
24 changes: 24 additions & 0 deletions site/scripts/build.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import chalk from "chalk";
import execa from "execa";

const script = async () => {
console.log(chalk.bold("Building..."));

await execa("yarn", ["build-blocks"], { stdio: "inherit" });

await (
await import("./generate-sitemap")
).default;

await (
await import("./generate-blocks-data")
).default;

await (
await import("./create-db-indexes")
).default;

await execa("next", ["build"], { stdio: "inherit" });
};

export default script();
7 changes: 5 additions & 2 deletions site/scripts/create-db-indexes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import chalk from "chalk";
import { User, UserDocument } from "../src/lib/model/user.model";
import {
VerificationCode,
Expand All @@ -18,6 +19,8 @@ const catchAndLog = async (func: () => Promise<void>) => {

// Actions done in the following script should be _idempotent_ as they run in dev/prod
const script = async () => {
console.log(chalk.bold("Creating DB indexes..."));

const { client, db } = await connectToDatabase();
await catchAndLog(async () => {
await db
Expand Down Expand Up @@ -47,7 +50,7 @@ const script = async () => {
});

await client.close();
console.log("✅ Created MongoDB indexes");
console.log("✅ MongoDB indexes created");
};

void script();
export default script();
22 changes: 22 additions & 0 deletions site/scripts/dev.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import chalk from "chalk";
import execa from "execa";

const script = async () => {
console.log(chalk.bold("Launching site in dev mode..."));

await (
await import("./generate-sitemap")
).default;

await (
await import("./generate-blocks-data")
).default;

await (
await import("./create-db-indexes")
).default;

await execa("next", ["dev"], { stdio: "inherit" });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

};

export default script();
19 changes: 13 additions & 6 deletions site/scripts/generate-blocks-data.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
import chalk from "chalk";
import { writeFileSync } from "fs";
import path from "path";
import { readBlocksFromDisk } from "../src/lib/blocks";

const blocksInfo = readBlocksFromDisk();
const script = async () => {
console.log(chalk.bold("Generating blocks data..."));

writeFileSync(
path.join(process.cwd(), `blocks-data.json`),
JSON.stringify(blocksInfo, null, "\t"),
);
const blocksInfo = readBlocksFromDisk();

console.log("✅ Generated blocks data");
writeFileSync(
path.join(process.cwd(), `blocks-data.json`),
JSON.stringify(blocksInfo, null, "\t"),
);

console.log("✅ Blocks data generated");
};

export default script();
19 changes: 13 additions & 6 deletions site/scripts/generate-sitemap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
import { writeFileSync } from "fs";
import path from "path";
import chalk from "chalk";
import { generateSiteMap } from "../src/lib/sitemap";

const sitemap = generateSiteMap();
const script = async () => {
console.log(chalk.bold("Generating sitemap..."));

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

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

console.log("✅ Site map generated");
};

export default script();
Comment on lines +6 to +19
Copy link
Contributor

@benwerner01 benwerner01 Feb 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@kachkaev kachkaev Feb 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

15 changes: 15 additions & 0 deletions site/scripts/prepare.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import chalk from "chalk";

const script = async () => {
console.log(chalk.bold("Preparing..."));

await (
await import("./generate-sitemap")
).default;

await (
await import("./generate-blocks-data")
).default;
};

export default script();
14 changes: 11 additions & 3 deletions site/scripts/seed-db.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import chalk from "chalk";
import { User, UserProperties } from "../src/lib/model/user.model";
import { ApiKey } from "../src/lib/model/apiKey.model";
import { VerificationCode } from "../src/lib/model/verificationCode.model";
import { connectToDatabase } from "../src/lib/mongodb";
import { EntityType } from "../src/lib/model/entityType.model";

void (async () => {
const script = async () => {
console.log(chalk.bold("Seeding DB..."));
const { client, db } = await connectToDatabase();

const existingCollections = await db.collections();
Expand Down Expand Up @@ -73,5 +75,11 @@ void (async () => {
);

await client.close();
console.log("✅ Seeded DB");
})();
console.log("✅ DB seeded");

await (
await import("./create-db-indexes")
).default;
};

export default script();
1 change: 1 addition & 0 deletions site/src/lib/mongodb.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ let cachedDb: Db | null = null;

export const connectToDatabase = async () => {
if (cachedClient && cachedDb) {
await cachedClient?.connect(); // Reconnect if client.close() was called previously
return { client: cachedClient, db: cachedDb };
}

Expand Down