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.

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

fix(server): ensure consistency between CLI serve entrypoints regarding help and strict #9809

Merged
merged 13 commits into from
Jan 19, 2024
2 changes: 1 addition & 1 deletion packages/api-server/src/cliHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export const apiCliOptions = {
port: { default: getConfig().api?.port || 8911, type: 'number', alias: 'p' },
socket: { type: 'string' },
apiRootPath: {
alias: ['rootPath', 'root-path'],
alias: ['api-root-path', 'rootPath', 'root-path'],
default: '/',
type: 'string',
desc: 'Root path where your api functions are served',
Expand Down
52 changes: 31 additions & 21 deletions packages/api-server/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#!/usr/bin/env node

import { hideBin } from 'yargs/helpers'
import yargs from 'yargs/yargs'

Expand All @@ -13,29 +14,38 @@ import {

export * from './types'

const positionalArgs = yargs(hideBin(process.argv)).parseSync()._

// "bin": {
// "rw-api-server-watch": "./dist/watch.js",
// "rw-log-formatter": "./dist/logFormatter/bin.js",
// "rw-server": "./dist/index.js"
// },

if (require.main === module) {
if (positionalArgs.includes('api') && !positionalArgs.includes('web')) {
apiServerHandler(
yargs(hideBin(process.argv)).options(apiCliOptions).parseSync()
yargs(hideBin(process.argv))
.scriptName('rw-server')
.usage('usage: $0 <side>')
.strict()

.command(
'$0',
'Run both api and web servers',
// @ts-expect-error just passing yargs though
(yargs) => {
yargs.options(commonOptions)
},
bothServerHandler
)
} else if (
positionalArgs.includes('web') &&
!positionalArgs.includes('api')
) {
webServerHandler(
yargs(hideBin(process.argv)).options(webCliOptions).parseSync()
.command(
'api',
'Start server for serving only the api',
// @ts-expect-error just passing yargs though
(yargs) => {
yargs.options(apiCliOptions)
},
apiServerHandler
)
} else {
bothServerHandler(
yargs(hideBin(process.argv)).options(commonOptions).parseSync()
.command(
'web',
'Start server for serving only the web side',
// @ts-expect-error just passing yargs though
(yargs) => {
yargs.options(webCliOptions)
},
webServerHandler
)
}
.parse()
}
14 changes: 12 additions & 2 deletions packages/cli/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,20 @@ async function runYargs() {
await loadPlugins(yarg)

// Run
await yarg.parse(process.argv.slice(2), {}, (_err, _argv, output) => {
await yarg.parse(process.argv.slice(2), {}, (err, _argv, output) => {
// Configuring yargs with `strict` makes it error on unknown args;
// here we're propagating the exit code.
jtoar marked this conversation as resolved.
Show resolved Hide resolved
if (err) {
process.exitCode = 1
Tobbe marked this conversation as resolved.
Show resolved Hide resolved
}

// Show the output that yargs was going to if there was no callback provided
if (output) {
console.log(output)
if (err) {
console.error(output)
} else {
console.log(output)
}
}
})
}
Expand Down
44 changes: 23 additions & 21 deletions packages/web-server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,14 @@ import path from 'path'
import chalk from 'chalk'
import { config } from 'dotenv-defaults'
import Fastify from 'fastify'
import yargsParser from 'yargs-parser'
import { hideBin } from 'yargs/helpers'
import yargs from 'yargs/yargs'

import { getPaths, getConfig } from '@redwoodjs/project-config'

import { redwoodFastifyWeb } from './web'
import { withApiProxy } from './withApiProxy'

interface Opts {
socket?: string
port?: string
apiHost?: string
}

function isFullyQualifiedUrl(url: string) {
try {
// eslint-disable-next-line no-new
Expand All @@ -29,22 +24,29 @@ function isFullyQualifiedUrl(url: string) {
}

async function serve() {
// Parse server file args
const args = yargsParser(process.argv.slice(2), {
string: ['port', 'socket', 'apiHost'],
alias: { apiHost: ['api-host'], port: ['p'] },
})

const options: Opts = {
socket: args.socket,
port: args.port,
apiHost: args.apiHost,
}
const options = yargs(hideBin(process.argv))
.scriptName('rw-web-server')
.usage('$0', 'Start server for serving only the web side')
.strict()

.options({
port: {
default: getConfig().web?.port || 8910,
type: 'number',
alias: 'p',
},
socket: { type: 'string' },
apiHost: {
alias: 'api-host',
type: 'string',
desc: 'Forward requests from the apiUrl, defined in redwood.toml to this host',
jtoar marked this conversation as resolved.
Show resolved Hide resolved
},
})
.parseSync()

const redwoodProjectPaths = getPaths()
const redwoodConfig = getConfig()

const port = options.port ? parseInt(options.port) : redwoodConfig.web.port
const apiUrl = redwoodConfig.web.apiUrl

if (!options.apiHost && !isFullyQualifiedUrl(apiUrl)) {
Expand Down Expand Up @@ -110,7 +112,7 @@ async function serve() {
listenOptions = { path: options.socket }
} else {
listenOptions = {
port,
port: options.port,
host: process.env.NODE_ENV === 'production' ? '0.0.0.0' : '::',
}
}
Expand All @@ -121,7 +123,7 @@ async function serve() {
if (options.socket) {
console.log(`Web server started on ${options.socket}`)
} else {
console.log(`Web server started on http://localhost:${port}`)
console.log(`Web server started on http://localhost:${options.port}`)
}
})

Expand Down
38 changes: 38 additions & 0 deletions tasks/server-tests/__snapshots__/server.test.mjs.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`serve web (/Users/dom/projects/redwood/redwood/packages/web-server/dist/server.js) errors out on unknown args 1`] = `
"rw-web-server

Start server for serving only the web side

Options:
--help Show help [boolean]
--version Show version number [boolean]
-p, --port [number] [default: 8910]
--socket [string]
--apiHost, --api-host Forward requests from the apiUrl, defined in
redwood.toml to this host [string]

Unknown arguments: foo, bar, baz
"
`;

exports[`serve web ([
'/Users/dom/projects/redwood/redwood/packages/api-server/dist/index.js',
'web'
]) errors out on unknown args 1`] = `
"rw-server web

Start server for serving only the web side

Options:
--help Show help [boolean]
--version Show version number [boolean]
-p, --port [number] [default: 8910]
--socket [string]
--apiHost, --api-host Forward requests from the apiUrl, defined in
redwood.toml to this host [string]

Unknown arguments: foo, bar, baz
"
`;
2 changes: 2 additions & 0 deletions tasks/server-tests/jest.config.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
/** @type {import('jest').Config} */
const config = {
rootDir: '.',
testMatch: ['<rootDir>/*.test.mjs'],
testTimeout: 5_000 * 2,
transform: {},
}

module.exports = config
Loading
Loading