Skip to content

Commit

Permalink
Remove custom server execution from the cli (#1096)
Browse files Browse the repository at this point in the history
* Remove custom server execution from the CLI

* Add a changeset

* Remove tests about --server arg

* Update docs and create-keystone-app
  • Loading branch information
emmatown authored and jesstelford committed May 8, 2019
1 parent 5637518 commit b22d6c1
Show file tree
Hide file tree
Showing 16 changed files with 68 additions and 119 deletions.
42 changes: 42 additions & 0 deletions .changeset/ae79fb47/changes.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
{
"releases": [
{ "name": "@keystone-alpha/demo-project-blog", "type": "patch" },
{ "name": "@keystone-alpha/demo-project-meetup", "type": "patch" },
{ "name": "@keystone-alpha/demo-project-todo", "type": "patch" },
{ "name": "@keystone-alpha/keystone", "type": "major" },
{ "name": "@keystone-alpha/cypress-project-access-control", "type": "patch" },
{ "name": "@keystone-alpha/cypress-project-basic", "type": "patch" },
{ "name": "@keystone-alpha/cypress-project-login", "type": "patch" },
{ "name": "@keystone-alpha/cypress-project-social-login", "type": "patch" }
],
"dependents": [
{
"name": "@keystone-alpha/adapter-knex",
"type": "patch",
"dependencies": ["@keystone-alpha/keystone"]
},
{
"name": "@keystone-alpha/adapter-mongoose",
"type": "patch",
"dependencies": ["@keystone-alpha/keystone"]
},
{
"name": "@keystone-alpha/test-utils",
"type": "patch",
"dependencies": [
"@keystone-alpha/adapter-knex",
"@keystone-alpha/adapter-mongoose",
"@keystone-alpha/keystone"
]
},
{
"name": "@keystone-alpha/api-tests",
"type": "patch",
"dependencies": [
"@keystone-alpha/adapter-mongoose",
"@keystone-alpha/test-utils",
"@keystone-alpha/keystone"
]
}
]
}
8 changes: 8 additions & 0 deletions .changeset/ae79fb47/changes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Remove custom server execution from the CLI.

The Keystone CLI does not execute custom servers anymore, instead of running `keystone` to start a Keystone instance that has a custom server, run the server file directly with `node`.

```diff
- "start": "keystone",
+ "start": "node server.js"
```
1 change: 1 addition & 0 deletions .changeset/f4ff3c5f/changes.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{ "releases": [{ "name": "create-keystone-app", "type": "minor" }], "dependents": [] }
1 change: 1 addition & 0 deletions .changeset/f4ff3c5f/changes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Update template to account for the removal of custom server execution in the Keystone CLI
10 changes: 4 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ Add a script to your `package.json`:
Create a file `index.js`:

<!-- prettier-ignore -->

```javascript
const { Keystone } = require('@keystone-alpha/keystone');
const { AdminUI } = require('@keystone-alpha/admin-ui');
Expand Down Expand Up @@ -162,7 +161,6 @@ must handle executing the different parts of Keystone.
Create the `server.js` file:

<!-- prettier-ignore -->

```javascript
const keystoneServer = require('@keystone-alpha/core');

Expand All @@ -178,10 +176,11 @@ keystoneServer.prepare({ port: 3000 })
});
```

Run keystone as you normally would:
You'll need to change the `dev` script in your `package.json` to run the server file with node like this.

```
npm run dev
```diff
- "dev": "keystone"
+ "dev": "node server.js"
```

#### Custom Server Configuration
Expand Down Expand Up @@ -266,7 +265,6 @@ To setup authentication, you must instantiate an _Auth Strategy_, and create a
list used for authentication in `index.js`:

<!-- prettier-ignore -->

```javascript
const { Keystone, PasswordAuthStrategy } = require('@keystone-alpha/keystone');
const { AdminUI } = require('@keystone-alpha/admin-ui');
Expand Down
2 changes: 1 addition & 1 deletion demo-projects/blog/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"node": ">=8.4.0"
},
"scripts": {
"start": "DISABLE_LOGGING=true keystone",
"start": "DISABLE_LOGGING=true node server.js",
"build": "next build"
},
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion demo-projects/meetup/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"node": ">=8.4.0"
},
"scripts": {
"start": "DISABLE_LOGGING=true keystone",
"start": "DISABLE_LOGGING=true node server.js",
"build": "next build"
},
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion demo-projects/todo/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"node": ">=8.4.0"
},
"scripts": {
"start": "DISABLE_LOGGING=true keystone"
"start": "DISABLE_LOGGING=true node server.js"
},
"dependencies": {
"@keystone-alpha/adapter-mongoose": "^1.0.6",
Expand Down
2 changes: 1 addition & 1 deletion packages/create-keystone-app/templates/todo/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@ The one 'extra' that this project includes is an example React App that consumes

## Custom Server

This project includes a _Custom Server_ in `server.js`. It is used to serve the react app only. If you wish to run Keystone as an API & AdminUI only, you can safely delete `server.js` (and the `/public` directory).
This project includes a _Custom Server_ in `server.js`. It is used to serve the react app only. If you wish to run Keystone as an API & AdminUI only, you can delete `server.js`, the `/public` directory and change the `start` script from running `node server.js` to running `keystone`
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "<%= appName%>",
"version": "0.0.0",
"scripts": {
"start": "DISABLE_LOGGING=true keystone"
"start": "DISABLE_LOGGING=true node server.js"
},
"private": true,
"dependencies": {
Expand Down
64 changes: 3 additions & 61 deletions packages/keystone/bin/commands/dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,55 +2,6 @@ const keystone = require('@keystone-alpha/core');
const endent = require('endent');
const path = require('path');

function getCustomServerFullPath(args, { exeName, _cwd }) {
const serverFile = args['--server'] ? args['--server'] : keystone.DEFAULT_SERVER;
try {
return Promise.resolve(require.resolve(path.resolve(_cwd, serverFile)));
} catch (error) {
if (args['--server']) {
// A custom server was specified, but it wasn't found
return Promise.reject(
new Error(endent`
--server=${serverFile} was passed to ${exeName}, but '${serverFile}' couldn't be found in ${process.cwd()}.
Ensure you're running ${exeName} from within the root directory of the project.
`)
);
}
// No custom server
return Promise.resolve();
}
}

function warnInvalidCustomServerArgs(args, { exeName }) {
if (args['--port']) {
console.warn(endent`
The ${exeName} --port CLI option does not work with a custom server.
To set the port, pass it to keystone from within your custom server:
> keystone.prepare({ port: 3000 })
`);
}

if (args['--entry']) {
console.warn(endent`
The ${exeName} --entry CLI option does not work with a custom server.
To set the entry file, pass it to keystone from within your custom server:
> keystone.prepare({ entryFile: 'index.js' })
`);
}
}

function executeCustomServer(serverFileFullPath) {
try {
// Let the custom server handle setup
require(serverFileFullPath);
return Promise.resolve();
} catch (error) {
// Something went wrong when requiring their custom server (eg; syntax
// error, etc). We make sure to expose the error here.
return Promise.reject(error);
}
}

function getEntryFileFullPath(args, { exeName, _cwd }) {
const entryFile = args['--entry'] ? args['--entry'] : keystone.DEFAULT_ENTRY;
try {
Expand Down Expand Up @@ -86,7 +37,6 @@ module.exports = {
'--port': Number,
'-p': '--port',
'--entry': String,
'--server': String,
},
help: ({ exeName }) => `
Usage
Expand All @@ -95,18 +45,10 @@ module.exports = {
Options
--port, -p Port to start on [${keystone.DEFAULT_PORT}]
--entry Entry file exporting keystone instance [${keystone.DEFAULT_ENTRY}]
--server Custom server file [${keystone.DEFAULT_SERVER}]
`,
exec: (args, { exeName, _cwd = process.cwd() } = {}) => {
return getCustomServerFullPath(args, { exeName, _cwd }).then(serverFile => {
if (serverFile) {
warnInvalidCustomServerArgs(args, { exeName });
return executeCustomServer(serverFile);
}

return getEntryFileFullPath(args, { exeName, _cwd }).then(entryFile =>
executeDefaultServer(args, entryFile)
);
});
return getEntryFileFullPath(args, { exeName, _cwd }).then(entryFile =>
executeDefaultServer(args, entryFile)
);
},
};
43 changes: 0 additions & 43 deletions packages/keystone/tests/bin/dev-command.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,49 +13,6 @@ describe('dev command', () => {
expect(typeof devCommand.help({ exeName: '' })).toBe('string');
});

describe('--server arg', () => {
test('rejects when file not found', () => {
expect(devCommand.exec({ '--server': 'foo.js', _cwd: __dirname })).rejects.toThrow(
/--server=.*was passed.*but.*couldn't be found/
);
});

test('requires custom server', async () => {
jest.resetModules();
const serverFileObj = tmp.fileSync({ postfix: '.js' });
let wasRequired = false;
jest.doMock(serverFileObj.name, () => {
wasRequired = true;
});
await devCommand.exec({ '--server': serverFileObj.name });
expect(wasRequired).toBeTruthy();
});

describe('warnings', () => {
afterEach(() => {
jest.restoreAllMocks();
});

test('warns when --port also set', async () => {
const mockWarn = jest.spyOn(global.console, 'warn').mockImplementation(jest.fn());
const serverFileObj = tmp.fileSync({ postfix: '.js' });
await devCommand.exec({ '--server': serverFileObj.name, '--port': 5000 });
expect(mockWarn).toHaveBeenLastCalledWith(
expect.stringContaining('--port CLI option does not work with a custom server')
);
});

test('warns when --entry also set', async () => {
const mockWarn = jest.spyOn(global.console, 'warn').mockImplementation(jest.fn());
const serverFileObj = tmp.fileSync({ postfix: '.js' });
await devCommand.exec({ '--server': serverFileObj.name, '--entry': 'foo.js' });
expect(mockWarn).toHaveBeenLastCalledWith(
expect.stringContaining('--entry CLI option does not work with a custom server')
);
});
});
});

describe('--entry arg', () => {
afterEach(() => {
jest.restoreAllMocks();
Expand Down
2 changes: 1 addition & 1 deletion test-projects/access-control/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"node": ">=8.4.0"
},
"scripts": {
"start": "node -r dotenv-safe/config `yarn bin`/keystone | sed -l -e 's/:\\s*undefined\\s*,/:null,/g' | tee out.log | pino-colada",
"start": "node -r dotenv-safe/config server.js | sed -l -e 's/:\\s*undefined\\s*,/:null,/g' | tee out.log | pino-colada",
"cypress:run:cmd": "node -r dotenv-safe/config `which cypress` run",
"cypress:open:cmd": "node -r dotenv-safe/config `which cypress` open",
"prepare-test-server": "NODE_ENV=test DISABLE_LOGGING=true node -r dotenv-safe/config -e 'require(`execa`)(`start-server-and-test`, [`start`, `http-get://localhost:${process.env.PORT}/admin`, process.argv[1]], { stdio: `inherit` }).catch(error => { console.error(error.toString()); process.exit(error.code) })'",
Expand Down
2 changes: 1 addition & 1 deletion test-projects/basic/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"node": ">=8.4.0"
},
"scripts": {
"start": "node -r dotenv-safe/config `yarn bin`/keystone | sed -l -e 's/:\\s*undefined\\s*,/:null,/g' | tee out.log | pino-colada",
"start": "node -r dotenv-safe/config server.js | sed -l -e 's/:\\s*undefined\\s*,/:null,/g' | tee out.log | pino-colada",
"cypress:run:cmd": "TZ=UTC node -r dotenv-safe/config `which cypress` run",
"cypress:open:cmd": "TZ=UTC node -r dotenv-safe/config `which cypress` open",
"prepare-test-server": "NODE_ENV=test DISABLE_LOGGING=true node -r dotenv-safe/config -e 'require(`execa`)(`start-server-and-test`, [`start`, `http-get://localhost:${process.env.PORT}/admin`, process.argv[1]], { stdio: `inherit` }).catch(error => { console.error(error.toString()); process.exit(error.code) })'",
Expand Down
2 changes: 1 addition & 1 deletion test-projects/login/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"node": ">=8.4.0"
},
"scripts": {
"start": "node -r dotenv-safe/config `yarn bin`/keystone | sed -l -e 's/:\\s*undefined\\s*,/:null,/g' | tee out.log | pino-colada",
"start": "node -r dotenv-safe/config server.js | sed -l -e 's/:\\s*undefined\\s*,/:null,/g' | tee out.log | pino-colada",
"cypress:run:cmd": "node -r dotenv-safe/config `which cypress` run",
"cypress:open:cmd": "node -r dotenv-safe/config `which cypress` open",
"prepare-test-server": "NODE_ENV=test DISABLE_LOGGING=true node -r dotenv-safe/config -e 'require(`execa`)(`start-server-and-test`, [`start`, `http-get://localhost:${process.env.PORT}/admin`, process.argv[1]], { stdio: `inherit` }).catch(error => { console.error(error.toString()); process.exit(error.code) })'",
Expand Down
2 changes: 1 addition & 1 deletion test-projects/social-login/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"node": ">=8.4.0"
},
"scripts": {
"start": "node -r dotenv-safe/config `yarn bin`/keystone | sed -l -e 's/:\\s*undefined\\s*,/:null,/g' | tee out.log | pino-colada",
"start": "node -r dotenv-safe/config server.js | sed -l -e 's/:\\s*undefined\\s*,/:null,/g' | tee out.log | pino-colada",
"cypress:run:ci": "exit 0",
"cypress:run": "exit 0"
},
Expand Down

0 comments on commit b22d6c1

Please sign in to comment.