Skip to content

Commit

Permalink
chore(examples/starter): Fix DB migrations and subcommand execution (#…
Browse files Browse the repository at this point in the history
…520)

This is a follow-up to #505
and #502. I'm addressing a
few different concerns in the same PR only because we don't have
automated tests for the starter and redoing the same manual testing
steps multiple times is error-prone.

Here's a brief description of changes.

## Removed `shelljs-live`

This reverts
a8764b0.

Turns out `shelljs-live` is not a drop-in replacement for `shelljs`. It
only uses shelljs's config but implements a different API for executing
a subprocess that only returns the status code to the caller. This broke
our error handling in `startCompose.js` and `startElectric.js` that
expects the return value to have the `stderr` field. I discovered this
problem when I executed `yarn backend:up` instead of `yarn
backend:start`:

$ yarn backend:up
yarn run v1.22.19
warning package.json: No license field
warning ../../../package.json: No license field
$ yarn backend:start --detach
warning package.json: No license field
warning ../../../package.json: No license field
$ node ./backend/startCompose.js --detach
[+] Running 5/5
 ✔ Network app_default         Created                                                                                                                                                                          0.1s 
 ✔ Volume "app_pg_data"        Created                                                                                                                                                                          0.0s 
 ✔ Volume "app_electric_data"  Created                                                                                                                                                                          0.0s 
 ✔ Container app-postgres-1    Started                                                                                                                                                                          0.1s 
 ✔ Container app-electric-1    Started                                                                                                                                                                          0.1s 
/home/alco/code/tmp/app/backend/startCompose.js:11
if (res.code !== 0 && res.stderr.includes('port is already allocated')) {
                                 ^

TypeError: Cannot read properties of undefined (reading 'includes')
    at Object.<anonymous> (/home/alco/code/tmp/app/backend/startCompose.js:11:34)
    at Module._compile (node:internal/modules/cjs/loader:1256:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
    at Module.load (node:internal/modules/cjs/loader:1119:32)
    at Module._load (node:internal/modules/cjs/loader:960:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:86:12)
    at node:internal/main/run_main_module:23:47

Node.js v18.18.0
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

It does mean we lose colorized output from `yarn backend:start`. But I
believe it can be restored by getting rid of `shelljs` completely and
using `cross-spawn` like `shelljs-live` does.

## Replaced ad-hoc migration runner with @databases/pg-migrations

This package does everything we need: it can run multiple migrations and
keeps track of already applied migrations.

The way `@databases/pg-migrations` keeps track of applied migrations is
a bit noisy:
```
[localhost] postgres:app=# \d
                          List of relations
 Schema │                 Name                  │   Type   │  Owner   
────────┼───────────────────────────────────────┼──────────┼──────────
 public │ atdatabases_migrations_applied        │ table    │ postgres
 public │ atdatabases_migrations_applied_id_seq │ sequence │ postgres
 public │ atdatabases_migrations_version        │ table    │ postgres
 public │ foo                                   │ table    │ postgres
 public │ items                                 │ table    │ postgres
(5 rows)
```

But I don't think it's a problem for the starter template. Once we
introduce integrations with backend frameworks, we'll replace this
migrations runner with framework-specific ones.

## Reset terminal color back to default after printing an error message

Our colorized error messages where missing the "reset" ANSI escape which
was causing the output following the error message to also be colored,
namely, the `Done` line printed by `yarn`.
  • Loading branch information
alco committed Oct 5, 2023
1 parent 0f065bc commit ef8dec9
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 53 deletions.
4 changes: 2 additions & 2 deletions examples/starter/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const spinner = ora('Validating arguments').start()

const error = (err: string) => {
spinner.stop()
console.error('\x1b[31m', err + '\nnpx create-electric-app [<app-name>] [--electric-port <port>] [--webserver-port <port>]')
console.error('\x1b[31m', err + '\nnpx create-electric-app [<app-name>] [--electric-port <port>] [--webserver-port <port>]', '\x1b[0m')
process.exit(1)
}

Expand Down Expand Up @@ -296,4 +296,4 @@ function parsePort(port: string): number {
error(`Invalid port '${port}. Port should be between 0 and 65535.'`)
}
return Number.parseInt(port)
}
}
9 changes: 5 additions & 4 deletions examples/starter/template/backend/startCompose.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
const exec = require('shelljs-live')
const shell = require('shelljs')
const path = require('path')

const envrcFile = path.join(__dirname, 'compose', '.envrc')
const composeFile = path.join(__dirname, 'compose', 'docker-compose.yaml')

const cliArguments = process.argv.slice(2).join(' ')

const res = exec(`docker compose --env-file ${envrcFile} -f ${composeFile} up ${cliArguments}`)
const res = shell.exec(`docker compose --env-file ${envrcFile} -f ${composeFile} up ${cliArguments}`)

if (res.code !== 0 && res.stderr.includes('port is already allocated')) {
// inform the user that they should change ports
console.error(
'\x1b[31m',
'Could not start Electric because the port seems to be taken.\n' +
'To run Electric on another port execute `yarn ports:configure`'
'To run Electric on another port execute `yarn ports:configure`',
'\x1b[0m'
)
}
}
11 changes: 6 additions & 5 deletions examples/starter/template/backend/startElectric.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const exec = require('shelljs-live')
const shell = require('shelljs')

let db = process.env.DATABASE_URL
let electricPort = process.env.ELECTRIC_PORT ?? 5133
Expand Down Expand Up @@ -50,7 +50,7 @@ const electric = process.env.ELECTRIC_IMAGE ?? "electricsql/electric:latest"

// 5433 is the logical publisher port
// it is exposed because PG must be able to connect to Electric
const res = exec(
const res = shell.exec(
`docker run \
-e "DATABASE_URL=${db}" \
-e "LOGICAL_PUBLISHER_HOST=localhost" \
Expand All @@ -64,11 +64,12 @@ if (res.code !== 0 && res.stderr.includes('port is already allocated')) {
console.error(
'\x1b[31m',
'Could not start Electric because the port seems to be taken.\n' +
'To run Electric on another port execute `yarn ports:configure`'
'To run Electric on another port execute `yarn ports:configure`',
'\x1b[0m'
)
}

function error(err) {
console.error('\x1b[31m', err + '\nyarn electric:start [-db <Postgres connection url>] [-p <Electric port>]')
console.error('\x1b[31m', err + '\nyarn electric:start [-db <Postgres connection url>] [-p <Electric port>]', '\x1b[0m')
process.exit(1)
}
}
5 changes: 3 additions & 2 deletions examples/starter/template/check-electric-is-running.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@ async function checkElectricIsRunning() {
console.error(
'\x1b[31m',
`Your application is configured to connect to Electric on port ${configuredPort} ` +
`but your instance of Electric is running on port ${port}`
`but your instance of Electric is running on port ${port}`,
'\x1b[0m'
)
process.exit(1)
}
}

checkElectricIsRunning()
checkElectricIsRunning()
4 changes: 3 additions & 1 deletion examples/starter/template/db/connect.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
const { DATABASE_URL, PUBLIC_DATABASE_URL } = require('./util.js')
const { spawn } = require('child_process')

console.info(`Connecting to postgres at ${PUBLIC_DATABASE_URL}`)
spawn(`psql ${DATABASE_URL}`, [], { cwd: __dirname, stdio: 'inherit', shell: true })

spawn("psql", [DATABASE_URL], { cwd: __dirname, stdio: 'inherit' })
55 changes: 20 additions & 35 deletions examples/starter/template/db/migrate.js
Original file line number Diff line number Diff line change
@@ -1,43 +1,28 @@
const fs = require('fs')
const path = require('path')
const { DATABASE_URL, PUBLIC_DATABASE_URL } = require('./util.js')

const createPool = require('@databases/pg')
const { sql } = require('@databases/pg')

const MIGRATIONS_DIR = process.env.MIGRATIONS_DIR || path.resolve(__dirname, 'migrations')
const { spawn } = require('child_process')
const process = require('process')

console.info(`Connecting to postgres at ${PUBLIC_DATABASE_URL}`)
const db = createPool(DATABASE_URL)

const apply = async (fileName) => {
const filePath = path.join(MIGRATIONS_DIR, fileName)
console.log('Applying', filePath)
const args = ["run", "-s", "pg-migrations", "apply", "--database", DATABASE_URL, "--directory", "./db/migrations"]
const proc = spawn("yarn", args, { cwd: __dirname })

await db.tx(
(tx) => tx.query(
sql.file(filePath)
)
)
}
let newMigrationsApplied = true

const main = async () => {
const fileNames = fs.readdirSync(MIGRATIONS_DIR)
for (const file of fileNames) {
if (path.extname(file) === '.sql') {
await apply(file)
}
proc.stdout.on('data', (data) => {
if (data.toString().trim() === 'No migrations required') {
newMigrationsApplied = false
} else {
process.stdout.write(data)
}
console.log('⚡️ Database migrated.')
}
})

try {
main()
}
catch (err) {
console.error(err)
process.exitCode = 1
}
finally {
db.dispose()
}
proc.on('exit', (code) => {
if (code === 0) {
if (newMigrationsApplied) {
console.log('⚡️ Database migrated.')
} else {
console.log('⚡ Database already up to date.')
}
}
})
4 changes: 2 additions & 2 deletions examples/starter/template/db/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const DATABASE_URL = process.env.DATABASE_URL || DEFAULT_URL
const PUBLIC_DATABASE_URL = DATABASE_URL.split('@')[1]

function error(err) {
console.error('\x1b[31m', err)
console.error('\x1b[31m', err, '\x1b[0m')
process.exit(1)
}

Expand Down Expand Up @@ -65,4 +65,4 @@ function fetchAppName() {

exports.DATABASE_URL = DATABASE_URL
exports.PUBLIC_DATABASE_URL = PUBLIC_DATABASE_URL
exports.fetchHostPortElectric = fetchHostPortElectric
exports.fetchHostPortElectric = fetchHostPortElectric
3 changes: 1 addition & 2 deletions examples/starter/template/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"wa-sqlite": "rhashimoto/wa-sqlite#master"
},
"devDependencies": {
"@databases/pg": "^5.4.1",
"@databases/pg-migrations": "^5.0.2",
"@electric-sql/prisma-generator": "latest",
"@prisma/client": "4.8.1",
"@types/node": ">=16.11.0",
Expand All @@ -40,7 +40,6 @@
"prisma": "4.8.1",
"prompt": "^1.3.0",
"shelljs": "^0.8.5",
"shelljs-live": "^0.0.5",
"tcp-port-used": "^1.0.2",
"typescript": "^4.4.3"
}
Expand Down

0 comments on commit ef8dec9

Please sign in to comment.