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

yarn rw dev fails when base path includes space #596

Closed
6 tasks
thedavidprice opened this issue May 25, 2020 · 3 comments
Closed
6 tasks

yarn rw dev fails when base path includes space #596

thedavidprice opened this issue May 25, 2020 · 3 comments
Labels
bug/confirmed We have confirmed this is a bug topic/cli

Comments

@thedavidprice
Copy link
Contributor

original forum topic "Yarn rw dev throwing AssertionError (Windows10)"

On both Windows and Mac (reproduced on my machine), when there is a space in the project path, yarn rw dev is throwing a Yargs assertion error that no command options are found in dev.js:

See Error Details
$ yarn rw dev                       
yarn run v1.22.4
$ '/Users/price/Repos/xx delete/xx-delete-again/node_modules/.bin/rw' dev
Dev BASE_DIR:  /Users/price/Repos/xx\ delete/xx-delete-again
Dev API_DIR:  /Users/price/Repos/xx\ delete/xx-delete-again/api
Dev WEB_DIR:  /Users/price/Repos/xx\ delete/xx-delete-again/web
rw dev [app..]

Run development servers for db, api, and web.

Options:
  --help     Show help                                                 [boolean]
  --version  Show version number                                       [boolean]
  --app              [choices: "db", "api", "web"] [default: ["db","api","web"]]

AssertionError [ERR_ASSERTION]: [concurrently] no commands provided
    at module.exports (/Users/price/Repos/redwoodjs-redwood/node_modules/concurrently/src/concurrently.js:24:12)
    at module.exports (/Users/price/Repos/redwoodjs-redwood/node_modules/concurrently/index.js:21:12)
    at Object.handler (/Users/price/Repos/redwoodjs-redwood/packages/cli/dist/commands/dev.js:80:29)
    at Object.runCommand (/Users/price/Repos/redwoodjs-redwood/node_modules/yargs/lib/command.js:240:40)
    at Object.parseArgs [as _parseArgs] (/Users/price/Repos/redwoodjs-redwood/node_modules/yargs/yargs.js:1154:41)
    at Object.get [as argv] (/Users/price/Repos/redwoodjs-redwood/node_modules/yargs/yargs.js:1088:21)
    at Object.<anonymous> (/Users/price/Repos/redwoodjs-redwood/packages/cli/dist/index.js:20:171)
    at Module._compile (internal/modules/cjs/loader.js:1156:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1176:10)
    at Module.load (internal/modules/cjs/loader.js:1000:32) {
  generatedMessage: false,
  code: 'ERR_ASSERTION',
  actual: 0,
  expected: 0,
  operator: 'notStrictEqual'
}
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

I confirmed all paths are correct per console.log:

BASE_DIR:  /Users/price/Repos/xx\ delete/xx-delete-again
API_DIR:  /Users/price/Repos/xx\ delete/xx-delete-again/api
WEB_DIR:  /Users/price/Repos/xx\ delete/xx-delete-again/web

Diagnoses

It seems the error was introduced in #490 with the use of fs.existsSync(). For our use, when checking a path including a space, it returns false. This explains why Yargs is complaining there are no commands.

Tests confirm the following:

  • given API_DIR = '/Users/price/Repos/xx\ delete/xx-delete-again/api'
  • fs.existsSync(API_DIR) returns false
    ...however...
  • 👉fs.existsSync('/Users/price/Repos/xx\ delete/xx-delete-again/api') returns true

So the method can handle spaces. It's just not happy with the way we are passing in the variable

I'm at a bit of a loss 🤔

Other CLI uses of fs.existsSync

This method is also used in the following commands. After determining a fix (or replacement), we should check the following and update as needed:

  • redwoodtools.js
  • build.js
  • auth.js
  • scaffold.js
  • index.js
  • create-redwood-app.js
@thedavidprice thedavidprice added bug/confirmed We have confirmed this is a bug topic/cli labels May 25, 2020
@peterp
Copy link
Contributor

peterp commented May 25, 2020

I think the problem is this line:

// For Windows: Replaces ` ` with `\ `. Damn, there has got to be a better
// way to sanitize paths?!
const BASE_DIR = getPaths().base.replace(' ', '\\ ')

We need to pass an unescaped version of BASE_DIR into the fs.existsSync command. I've recently added getPaths().api.base and getPaths().web.base, so we could use that instead of the "sanitised version."

I think the root cause here is with concurrently, since it doesn't allow us to programatically set the cwd, we pass in a command like so: cd ${API_DIR} && yarn dev-server

I added an issue to concurrently.

@thedavidprice
Copy link
Contributor Author

The error is coming from these lines:

runWhen: () => fs.existsSync(API_DIR),

runWhen: () => fs.existsSync(API_DIR),

runWhen: () => fs.existsSync(WEB_DIR),

For some reason, fs.existsSync(X_DIR) always returns false if the variable path contains a space (or an escaped space, i.e. \ ). This is why the Yargs command option array ends up being empty -- no jobs are added to the run on this line:

.filter((job) => job && job.runWhen()),

Update

I'm making some progress using getPaths().api.base and getPaths().web.base. Need to iron out a few remaining issues and then test on Windows.

@peterp
Copy link
Contributor

peterp commented Jun 4, 2020

@thedavidprice I think this was fixed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/confirmed We have confirmed this is a bug topic/cli
Projects
None yet
Development

No branches or pull requests

2 participants