sheerun / knex-migrate

Modern database migration toolkit for knex.js
MIT License
342 stars 39 forks source link

No knexfile at ... #29

Open dvisztempacct opened 6 years ago

dvisztempacct commented 6 years ago

The "No knexfile at" error message is confusing when the actual problem is a "Cannot find module" error occurring deeper in the recursive module resolution of your knexfile.js.

For example, create a knexfile.js and add to it require("module-that-does-not-exist")

knex-migrate will tell you that you:

No knex file at /path/to/your/project/knexfile.js

Which isn't exactly true.

sheerun commented 6 years ago

The knexfile is supposed to be exactly at /path/to/your/project/knexfile.js. Sorry if code suggests otherwise, but the message is correct.

tataton commented 6 years ago

I think this isn't what the original commenter meant. If knex-migrate fails to resolve an import within knexfile.js, it reports:

No knexfile at *dirname*; Please create one or bootstrap using 'knex init'

It does this even though knexfile.js exists and is located correctly. So, for example, for:

_knexfile.js_

const config = require('config');
module.exports = {
  client: 'pg',
  connection: { user: config.get('DB_USER'), database: config.get('DB_NAME') }
};

If I've failed to install config, I get the "No knexfile" error. I understand this is a simplistic example (isn't failing to install config my fault after all?), but if your knexfile.js contains some logic that templates the export object from different sources/imports, you could imagine a trace being useful for debugging here.

It looks like this is a consequence of a regex test in src/index.js that reports all "cannot find module" errors as being due to a missing knexfile. Could this test be made more narrow, so that more errors break thru to throw err?

Thx, helpful package!

sheerun commented 6 years ago

Yes, I understand it now. This needs to be fixed

dvisztempacct commented 6 years ago

@tataton thanks for the clarification. My original post was a little confusing...

tomasikp commented 4 years ago

+1 for this