[now dev] Allow now dev as package.json script (#2559)

* [now dev] Allow `now dev` as package.json script

* Fix absolute path for now dev

* Fix directory issue

* Typo

* Update src/commands/dev/dev.ts

Co-Authored-By: Nathan Rajlich <n@n8.io>

* Whitespace

* Make code simpler

* Extend type

* Update errors/now-dev-as-dev-script.md

Co-Authored-By: Nathan Rajlich <n@n8.io>

* Update errors/now-dev-as-dev-script.md

Co-Authored-By: Nathan Rajlich <n@n8.io>

errors/now-dev-as-dev-script.md

@@ -0,0 +1,9 @@
+# `now dev` as `dev` script
+
+#### Why This Error Occurred
+
+The `package.json` file of the used project invokes `now dev` as `dev` script. This would cause `now dev` to recursively invoke itself.
+
+#### Possible Ways to Fix It
+
+Adjust the `dev` script inside the `package.json` file to match what your framework uses to begin development mode, e.g. `next` for Next.js or `gatsby develop` for Gatsby.

src/commands/dev/dev.ts

@@ -24,7 +24,7 @@ export default async function dev(
   );
 
   const [dir = '.'] = args;
-  const cwd = path.join(process.cwd(), dir);
+  const cwd = path.resolve(dir);
   const port = opts['-p'] || opts['--port'];
   const debug = opts['-d'] || opts['--debug'];
   const devServer = new DevServer(cwd, { output, debug });

src/commands/dev/index.ts

@@ -1,16 +1,18 @@
+import path from 'path';
 import chalk from 'chalk';
 
 import getArgs from '../../util/get-args';
 import getSubcommand from '../../util/get-subcommand';
-import { NowContext } from '../../types';
+import { NowContext, Config } from '../../types';
 import { NowError } from '../../util/now-error';
 import handleError from '../../util/handle-error';
 import createOutput from '../../util/output/create-output';
 import logo from '../../util/output/logo';
 import cmd from '../../util/output/cmd';
 import dev from './dev';
-import readPackage from '../../util/read-package'
+import readPackage from '../../util/read-package';
 import { Package } from '../../util/dev/types';
+import readConfig from '../../util/config/read-config';
 
 const COMMAND_CONFIG = {
   dev: ['dev']
@@ -58,13 +60,23 @@ export default async function main(ctx: NowContext) {
     return 2;
   }
 
-  const pkg = await readPackage();
+  const [dir = '.'] = args;
-  if (pkg) {
-    const { scripts } = pkg as Package;
 
-    if (scripts && scripts.dev && /\bnow\b\W+\bdev\b/.test(scripts.dev)) {
+  const nowJson = await readConfig(path.join(dir, 'now.json'));
-      output.error(`The ${cmd('dev')} script in ${cmd('package.json')} must not contain ${cmd('now dev')}`);
+  // @ts-ignore: Because `nowJson` could be one of three different types
-      return 1;
+  const hasBuilds = nowJson && nowJson.builds && nowJson.builds.length > 0;
+
+  if (!nowJson || !hasBuilds) {
+    const pkg = await readPackage(path.join(dir, 'package.json'));
+
+    if (pkg) {
+      const { scripts } = pkg as Package;
+
+      if (scripts && scripts.dev && /\bnow\b\W+\bdev\b/.test(scripts.dev)) {
+        output.error(`The ${cmd('dev')} script in ${cmd('package.json')} must not contain ${cmd('now dev')}`);
+        output.error(`More details: http://err.sh/now-cli/now-dev-as-dev-script`);
+        return 1;
+      }
     }
   }
 

src/types.ts

@@ -1,6 +1,8 @@
+import { NowConfig } from './util/dev/types';
+
 export type ThenArg<T> = T extends Promise<infer U> ? U : T;
 
-export interface Config {
+export interface Config extends NowConfig {
   alias?: string[] | string;
   aliases?: string[] | string;
   name?: string;

src/util/config/read-config.ts

@@ -0,0 +1,20 @@
+import path from 'path';
+import { CantParseJSONFile } from '../errors-ts';
+import readJSONFile from '../read-json-file';
+import { Config } from '../../types';
+import getLocalConfigPath from './local-path';
+
+export default async function readConfig(file?: string) {
+  const pkgFilePath = file || getLocalConfigPath(process.cwd());
+  const result = await readJSONFile(pkgFilePath);
+
+  if (result instanceof CantParseJSONFile) {
+    return result;
+  }
+
+  if (result) {
+    return result as Config;
+  }
+
+  return null;
+}

src/util/read-package.ts

@@ -11,14 +11,14 @@ type Package = {
 export default async function readPackage(file?: string) {
   const pkgFilePath = file || path.resolve(process.cwd(), 'package.json');
   const result = await readJSONFile(pkgFilePath);
+
   if (result instanceof CantParseJSONFile) {
     return result;
   }
 
-  if (result !== null) {
+  if (result){
-    const pkg = result as Package;
+    return result as Package
-    return pkg;
   }
 
-  return result;
+  return null;
 }

test/helpers/prepare.js

@@ -198,6 +198,13 @@ RUN echo $NONCE > /public/index.html
     'now-revert-alias-2': {
       'index.json': JSON.stringify({ name: 'now-revert-alias-2' }),
       'now.json': getRevertAliasConfigFile()
+    },
+    'now-dev-fail-dev-script': {
+      'package.json': JSON.stringify({
+        scripts: {
+          dev: 'now dev'
+        }
+      }, null, 2)
     }
   };
 

test/integration.js

@@ -31,6 +31,9 @@ const pickUrl = stdout => {
   return lines[lines.length - 1];
 };
 
+const createFile = dest => fs.closeSync(fs.openSync(dest, 'w'));
+const createDirectory = dest => fs.mkdirSync(dest);
+
 const waitForDeployment = async href => {
   // eslint-disable-next-line
   while (true) {
@@ -1255,8 +1258,6 @@ test('try to deploy with non-existing team', async t => {
   t.true(stderr.includes(goal));
 });
 
-const createFile = dest => fs.closeSync(fs.openSync(dest, 'w'));
-const createDirectory = dest => fs.mkdirSync(dest);
 const verifyExampleApollo = (cwd, dir) =>
   fs.existsSync(path.join(cwd, dir, 'package.json')) &&
   fs.existsSync(path.join(cwd, dir, 'now.json')) &&
@@ -1426,6 +1427,14 @@ test('try to update now to canary', async t => {
   t.true(stdout.includes('canary'));
 });
 
+test('fail `now dev` dev script without now.json', async t => {
+  const deploymentPath = fixture('now-dev-fail-dev-script');
+  const { code, stderr } = await execute(['dev', deploymentPath]);
+
+  t.is(code, 1);
+  t.true(stderr.includes('must not contain `now dev`'), `Received instead: "${stderr}"`);
+});
+
 test.after.always(async () => {
   // Make sure the token gets revoked
   await execa(binaryPath, ['logout', ...defaultArgs]);