Overhaul monorepo tooling #201

versecafe · 2024-08-22T19:02:28Z

Checklist

NVM version file
Turbo repo build tools
Turbo cache policies
GitHub Actions CI
Turbo caching on CI (just a quick change to CI)
Remove script mess
Compile packages cleanly (just cleaner not redoing full build system)

benjreinhart · 2024-08-22T19:13:06Z

I'm all for improving the monorepo setup (it's in a poor state right now), but why do we need to switch to Turbo repo over pnpm? What benefits would that deliver and what are the cons?

versecafe · 2024-08-22T19:21:38Z

Turbo is a layer on top of pnpm to start so the core remains, but the main benefits are dependency tracking in builds so package A depends on B used by app C, second is caching policies so avoiding continual rebuilds but tracking when a rebuild is needed for all downstream dependents, also applies to things like type checks, tests, etc. also provides a clear set structure for adding new tasks and making sure they get linked together along with a good TUI for looking at logs and behaviour across a few builds at once for example, other smaller wins include remote caching if configured. especially becomes more helpful as things scale for example having 20 plugin packages at some point

versecafe · 2024-08-22T20:10:23Z

Example of smoother process so far, synced branch with main, git pull, and then turbo start and it all just works in under a second

remote: Compressing objects: 100% (6/6), done.
remote: Total 12 (delta 6), reused 10 (delta 6), pack-reused 0 (from 0)
Unpacking objects: 100% (12/12), 4.42 KiB | 411.00 KiB/s, done.
From https://github.com/versecafe/srcbook
   b241854..10859aa  improve-monorepo-handling -> origin/improve-monorepo-handling
Updating b241854..10859aa
Fast-forward
 packages/api/test/tsserver.test.mts |  94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 packages/api/tsserver/messages.mts  | 134 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 packages/api/tsserver/tsserver.mts  |   4 +--
 packages/api/tsserver/utils.mts     |  54 ----------------------------------------
 4 files changed, 230 insertions(+), 56 deletions(-)
 create mode 100644 packages/api/test/tsserver.test.mts
 create mode 100644 packages/api/tsserver/messages.mts
veronica@Veronicas-MacBook-Pro srcbook % clear
veronica@Veronicas-MacBook-Pro srcbook % turbo start
turbo 2.0.14

• Packages in scope: @srcbook/api, @srcbook/shared, @srcbook/web, srcbook
• Running start in 4 packages
• Remote caching disabled
@srcbook/shared:build: cache hit, replaying logs bae3e21fc8704df8
@srcbook/shared:build:
@srcbook/shared:build:
@srcbook/shared:build: > @srcbook/[email protected] prebuild /Users/veronica/Projects/srcbook/packages/shared
@srcbook/shared:build: > rm -rf ./dist
@srcbook/shared:build:
@srcbook/shared:build:
@srcbook/shared:build: > @srcbook/[email protected] build /Users/veronica/Projects/srcbook/packages/shared
@srcbook/shared:build: > tsc
@srcbook/shared:build:
srcbook:start: cache miss, executing cb19eb9641c11dfd
srcbook:start:
srcbook:start:
srcbook:start: > [email protected] start /Users/veronica/Projects/srcbook/srcbook
srcbook:start: > ./bin/cli.mjs start
srcbook:start:
  Srcbook
srcbook:start: Serving static files (React app)...
srcbook:start: Creating WebSocket server...
srcbook:start: Initialization complete
srcbook:start: [email protected] running at http://localhost:2150

benjreinhart · 2024-08-22T20:34:07Z

Sweet! I'm not too opinionated on the build tooling / monorepo setup, so if this delivers tangible benefits with minimal maintenance overhead, I'm excited to get it checked in!

🙏🏻

versecafe · 2024-08-22T20:43:39Z

@benjreinhart Are you okay if I setup proper packaging instead of doing a pile of tsc and cp moves to make things easier to track as regular packages since the way @srcbook/api is setup right now makes it hard to integrate properly with dependency resolution

benjreinhart · 2024-08-22T20:50:55Z

Yes, that would be great. Most of our tooling was setup in a rush without much thought so if you see concrete areas of improvement I'm happy to accept.

versecafe · 2024-08-22T20:52:24Z

Alright great, thank you for the quick feedback

versecafe · 2024-08-23T20:26:46Z

@benjreinhart Can I get a quick meeting with you to go over restructuring the internal packages inside srcbook/api to finish up the final steps of this

versecafe · 2024-08-23T21:43:29Z

Officially ready to go as simple as

turbo dev
# or
turbo start

nichochar

This is a great improvement, over all of the relative path and custom building.

(Let's wait for @benjreinhart stamp to merge though, he is running final checks on building)

README.md

benjreinhart

This looks super awesome. Everything for dev seems 👌🏻 . Still need to test that the builds and deploys to NPM do everything expected (e.g., migrations), but I'm expecting this is to work great.

Thank you so much for this, will follow up with a once we test the npm deploys.

🙏🏻

packages/web/package.json

package.json

benjreinhart

Found two more issues. I think once these get fixed we'll likely be good.

My acceptance criteria here includes:

If I run pnpm start from a clean slate (no ~/.srcbook directory), it should work.
If I have an existing ~/srcbook directory with srcbook state and run pnpm start, it should work and I should see all my existing srcbooks / data.

benjreinhart · 2024-08-24T23:00:21Z

packages/web/package.json

@@ -6,7 +6,7 @@
  "scripts": {
    "dev": "vite",
    "prebuild": "rm -rf ./dist",
-    "build": "tsc && vite build",
+    "build": "tsc && vite build && cp -R ./dist ../../srcbook/public",


This copies dist into public such that it's public/dist/{files} but the express server is expecting public/{files}.

I think changing it to cp -R ./dist/. ../../srcbook/public will work

benjreinhart · 2024-08-24T23:04:10Z

srcbook/src/server.mjs


-// This is important. It will create and setup the database. Import this second.
-import '../lib/db/index.mjs';
+configureDB();


I think something with these changes has caused an error when starting srcbook for the first time. If I delete my ~/.srcbook directory, and run pnpm start, then I see the following:

srcbook:start: cache miss, executing b8b61b96f22ab5f1 srcbook:start: srcbook:start: srcbook:start: > [email protected] start /Users/ben/Desktop/axflow/srcbook/srcbook srcbook:start: > ./bin/cli.mjs start srcbook:start: srcbook:start: /Users/ben/Desktop/axflow/srcbook/node_modules/.pnpm/[email protected]/node_modules/better-sqlite3/lib/database.js:65 srcbook:start: throw new TypeError('Cannot open database because the directory does not exist'); srcbook:start: ^ srcbook:start: srcbook:start: TypeError: Cannot open database because the directory does not exist srcbook:start: at new Database (/Users/ben/Desktop/axflow/srcbook/node_modules/.pnpm/[email protected]/node_modules/better-sqlite3/lib/database.js:65:9) srcbook:start: at file:///Users/ben/Desktop/axflow/srcbook/packages/api/dist/db/index.mjs:15:16 srcbook:start: at ModuleJob.run (node:internal/modules/esm/module_job:262:25) srcbook:start: at async ModuleLoader.import (node:internal/modules/esm/loader:474:24) srcbook:start: at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:119:5) srcbook:start: srcbook:start: Node.js v22.1.0 srcbook:start: ELIFECYCLE Command failed with exit code 1. srcbook:start: ERROR: command finished with error: command (/Users/ben/Desktop/axflow/srcbook/srcbook) /Users/ben/.nvm/versions/node/v22.1.0/bin/pnpm run start exited (1) srcbook#start: command (/Users/ben/Desktop/axflow/srcbook/srcbook) /Users/ben/.nvm/versions/node/v22.1.0/bin/pnpm run start exited (1)

Unfortunately, I remember this being a weird sequence of events to get right when we coded this up. Perhaps that's what ended up making some of this code confusing / gross.

@benjreinhart Most annoying thing ever to debug relative import order through compilation and how it gets ranked but solved it by doing initialization inside db before creating the db instance like so

import path from 'node:path'; import { drizzle } from 'drizzle-orm/better-sqlite3'; import Database from 'better-sqlite3'; import * as schema from './schema.mjs'; import { migrate } from 'drizzle-orm/better-sqlite3/migrator'; import { DIST_DIR, SRCBOOKS_DIR } from '../constants.mjs'; import fs from 'node:fs'; // We can't use a relative directory for drizzle since this application // can get run from anywhere, so use DIST_DIR as ground truth. const drizzleFolder = path.join(DIST_DIR, 'drizzle'); const DB_PATH = `${process.env.HOME}/.srcbook/srcbook.db`; // Creates the HOME/.srcbook/srcbooks dir fs.mkdirSync(SRCBOOKS_DIR, { recursive: true }); export const db = drizzle(new Database(DB_PATH), { schema }); migrate(db, { migrationsFolder: drizzleFolder });

benjreinhart

I haven't attempted an npm publish yet, but I feel pretty confident about these changes and everything is working great locally.

Thank you for this!!

benjreinhart

Whoops, I just noticed that the dev command (pnpm run dev) is not working. I think it's cause of the deletion of initialization.mts.

versecafe · 2024-08-25T20:58:03Z

Whoops, I just noticed that the dev command (pnpm run dev) is not working. I think it's cause of the deletion of initialization.mts.

@benjreinhart Fixed it missed that because it's a call on a pre script to another scrip, fixed now my bad

nichochar · 2024-08-26T17:32:31Z

I tried deploying and have found a weird issue: the srcbook package thinks that a thing is missing from @srcbook/shared, but it's not in there. I'm assuming it's pointing to an older build with outdated dependencies.

Debugging now, but wanted to add convo here for context:

file:///Users/nicholascharriere/.nvm/versions/node/v22.1.0/lib/node_modules/scratch-srcbook/node_modules/@srcbook/api/dist/server/ws.mjs:7
import { CellExecPayloadSchema, CellStopPayloadSchema, DepsInstallPayloadSchema, DepsValidatePayloadSchema, CellUpdatedPayloadSchema, CellOutputPayloadSchema, DepsValidateResponsePayloadSchema, CellStdinPayloadSchema, CellUpdatePayloadSchema, } from '@srcbook/shared';
                                                                                                                                                                                                  ^^^^^^^^^^^^^^^^^^^^^^
SyntaxError: The requested module '@srcbook/shared' does not provide an export named 'CellStdinPayloadSchema'
    at ModuleJob._instantiate (node:internal/modules/esm/module_job:171:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:254:5)
    at async ModuleLoader.import (node:internal/modules/esm/loader:474:24)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:119:5)

Update: I see the issue. This now requires us to deploy @srcbook/api, where previously it was vendored..
This was intentional at the time, to simplify our package to be a mostly-single-binary (still depended on shared). Will deploy @srcbook/api but we might want to sit down and rethink our bundling strategy for rolling things out.

benjreinhart · 2024-08-26T18:10:47Z

Deploying the api package aside, I don't understand why that error happened but I would also use --force when running the build command here which should not use any cache.

versecafe · 2024-08-26T18:26:57Z

I can definitely look into it, I haven't seen the issue yet but always try running force as a fallback, but this means some set of the dependency tracking might not be working.

add nvmrc to standardize node version

7490397

versecafe added 4 commits August 22, 2024 14:30

simple build & check types setup

986f242

pnpm to newest version

8e45ee6

support for turbo start to build if needed and start

b241854

Merge branch 'main' into improve-monorepo-handling

10859aa

update readme requirements to meet new setup

bc7da49

versecafe added 3 commits August 22, 2024 16:04

functional git workflow for lint, build, test

82a392a

overhaul workflow for proper dep handling

f2fd2d9

finally get CI to behave

418da23

versecafe added 4 commits August 23, 2024 16:34

full build and start pipeline handling and removed scripts

d6ac2c4

add commented out remote CI caching config for future

a98256f

update contributing & get turbo dev working

5006249

Merge branch 'main' into improve-monorepo-handling

a144ecd

versecafe marked this pull request as ready for review August 23, 2024 21:43

nichochar approved these changes Aug 23, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

benjreinhart reviewed Aug 23, 2024

View reviewed changes

packages/web/package.json Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

package.json Show resolved Hide resolved

package.json Show resolved Hide resolved

versecafe added 2 commits August 23, 2024 17:16

update requirements

54f26ba

resolve PR feedback

4f95817

benjreinhart reviewed Aug 24, 2024

View reviewed changes

versecafe added 2 commits August 24, 2024 19:21

update web build export for express

daa0a86

fixes initialization sequences of srcbook dir and db

17dfc93

benjreinhart approved these changes Aug 25, 2024

View reviewed changes

benjreinhart requested changes Aug 25, 2024

View reviewed changes

cleanup some now uneeded scripts

ebeb9a1

benjreinhart approved these changes Aug 25, 2024

View reviewed changes

benjreinhart merged commit 38b0392 into srcbookdev:main Aug 25, 2024
1 check passed

versecafe deleted the improve-monorepo-handling branch August 25, 2024 21:55

This was referenced Aug 27, 2024

fix: Limit title length to 50 characters in cellUpdate function #212

Merged

CI doesn't seem to run FE linter #236

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Overhaul monorepo tooling #201

Overhaul monorepo tooling #201

versecafe commented Aug 22, 2024 •

edited

Loading

benjreinhart commented Aug 22, 2024

versecafe commented Aug 22, 2024

versecafe commented Aug 22, 2024

benjreinhart commented Aug 22, 2024

versecafe commented Aug 22, 2024

benjreinhart commented Aug 22, 2024

versecafe commented Aug 22, 2024

versecafe commented Aug 23, 2024

versecafe commented Aug 23, 2024

nichochar left a comment •

edited

Loading

benjreinhart left a comment

benjreinhart left a comment

benjreinhart Aug 24, 2024

benjreinhart Aug 24, 2024

versecafe Aug 25, 2024

benjreinhart left a comment

benjreinhart left a comment

versecafe commented Aug 25, 2024

nichochar commented Aug 26, 2024 •

edited

Loading

benjreinhart commented Aug 26, 2024

versecafe commented Aug 26, 2024

Overhaul monorepo tooling #201

Overhaul monorepo tooling #201

Conversation

versecafe commented Aug 22, 2024 • edited Loading

Checklist

benjreinhart commented Aug 22, 2024

versecafe commented Aug 22, 2024

versecafe commented Aug 22, 2024

benjreinhart commented Aug 22, 2024

versecafe commented Aug 22, 2024

benjreinhart commented Aug 22, 2024

versecafe commented Aug 22, 2024

versecafe commented Aug 23, 2024

versecafe commented Aug 23, 2024

nichochar left a comment • edited Loading

Choose a reason for hiding this comment

benjreinhart left a comment

Choose a reason for hiding this comment

benjreinhart left a comment

Choose a reason for hiding this comment

benjreinhart Aug 24, 2024

Choose a reason for hiding this comment

benjreinhart Aug 24, 2024

Choose a reason for hiding this comment

versecafe Aug 25, 2024

Choose a reason for hiding this comment

benjreinhart left a comment

Choose a reason for hiding this comment

benjreinhart left a comment

Choose a reason for hiding this comment

versecafe commented Aug 25, 2024

nichochar commented Aug 26, 2024 • edited Loading

benjreinhart commented Aug 26, 2024

versecafe commented Aug 26, 2024

versecafe commented Aug 22, 2024 •

edited

Loading

nichochar left a comment •

edited

Loading

nichochar commented Aug 26, 2024 •

edited

Loading