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

Knex cli should support es6 for configuration and migrations #1232

Closed
olalonde opened this issue Feb 26, 2016 · 42 comments
Closed

Knex cli should support es6 for configuration and migrations #1232

olalonde opened this issue Feb 26, 2016 · 42 comments

Comments

@olalonde
Copy link

It would be nice if the knex cli would support es6 for configuration and migrations. Maybe this could be behind a flag or only when there is a .babelrc file in current working directory.

@rhys-vdw
Copy link
Member

rhys-vdw commented Mar 6, 2016

Definitely this would be great. Happy to accept a PR for this one.

@ngoctranfire
Copy link
Contributor

Sorry. That previous comment was supposed to be for issue 1220, but I mistakenly put that comment here.

@jmfurlott
Copy link

@rhys-vdw do you know off hand what it would take?

@ngoctranfire
Copy link
Contributor

@rhys-vdw @jmfurlott . I'm actually interested in pursuing this too as I find this would be a really cool feature to have. Do you think you can point us to the components we should explore to try and make this a possibility. Some insight would help

@olalonde
Copy link
Author

olalonde commented Apr 26, 2016

Adding require("babel-register") to the CLI entry point might just work.

@stubailo
Copy link

Confirmed - added that to my seed file and had ES2015 running! Maybe should be an option in knexfile.js to set compiler, like mocha does with their CLI?

@n1ru4l
Copy link

n1ru4l commented Jun 17, 2016

I finally got mine working:

db.config.js

export default {
    devDB: {
        client: 'sqlite3',
        connection: {
            filename: './db.sqlite3',
        },
        pool: {
            min: 1,
            max: 1,
        },
    },
    migrations: {
        directory: './migrations',
        tableName: '_migrations',
    },
    seeds: {
        directory: './seeds/dev'
    },
    pool: {
        min: 1,
        max: 1,
    }
}

knexfile.js

require('babel-register')
const config = require('./db.config').default // <- this -.-

module.exports = config

.babelrc

{
    "env": {
        "devDB": {
            "plugins": ["transform-es2015-modules-commonjs"]
        }
    }
}

However i can not change the seed path, when they are not located in ./seeds i will always get

No seed files exist

Same also for migrations..

@alecmev
Copy link

alecmev commented Jan 11, 2017

Another option is to run Knex with babel-node, available in babel-cli, like so:

{
  "scripts": {
    "db:migrate:latest": "babel-node node_modules/.bin/knex migrate:latest"
  }
}

You might want to do this if your app imports knexfile.js while already being processed by Babel, rendering the require('babel-register') option unusable.

@olalonde
Copy link
Author

@jeremejevs good idea... with

{
  "scripts": {
    "knex": "babel-node node_modules/.bin/knex"
  }
}

you could use it this way

npm run knex migrate:latest

without having to create a separate script for all Knex commands.

@alecmev
Copy link

alecmev commented Jan 11, 2017

@olalonde Yep, that works too! I just prefer having explicit "db:migrate:make" and "db:migrate:latest" scripts in case I forget the exact wording half a year later (was it "make"? or "make-migration"? or "migrate:make"? or "migrate:new"? or...) 🙂

@sirgalleto
Copy link

@olalonde excellent proposal! Then you can use the npm scripts "composition" like:

{
    "knex": "babel-node node_modules/.bin/knex",
    "migrate": "npm run knex -- migrate:latest --env ",
}

@n1ru4l
Copy link

n1ru4l commented Aug 15, 2017

Maybe this could help aswell: https://github.com/standard-things/esm

@cmd-shift-s
Copy link

cmd-shift-s commented Sep 13, 2017

OS: windows
babel-cli@6.26.0
knex@0.13.0

// migrations/contacts.js
export function up (knex) {
  return knex.schema
    .createTable('contacts', table => {
      table.increments('id').primary()
      table.string('firstName')
      table.string('lastName')
      table.string('emailAddress')
    })
}

export function down(knex) {
  return knex.schema
    .dropTable('contacts')
}
$ npx knex migrate:latest
Using environment: development
migrations/contacts.js:1
(function (exports, require, module, __filename, __dirname) { export function up (knex) {
                                                              ^^^^^^
SyntaxError: Unexpected token export
    at Object.exports.runInThisContext (vm.js:73:16)
    at Module._compile (module.js:543:28)
    ...
$ npx babel-node node_modules/.bin/knex migrate:latest
./node_modules/.bin/knex:2
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")
          ^^^^^^^
SyntaxError: missing ) after argument list
    at Object.exports.runInThisContext (vm.js:73:16)
    at Module._compile (module.js:543:28)
    ...
$ npx babel-node node_modules/knex/bin/cli.js migrate:latest
Using environment: development
Batch 1 run: 1 migrations
migrations/contacts.js

@hagabaka
Copy link

When I try @olalonde's suggestion, npm run knex migrate:make, or npm run knex with any additional arguments, just exits without displaying anything. Just running npm run knex displays the usage as normal though. npm docs say that I need to put -- before the script arguments like npm run knex -- migrate:make, but that still exits with no output.

@ghost
Copy link

ghost commented Feb 22, 2018

Using reify (fast!) on *nix (no idea about an alias synonym on windows, sorry!)

/*

  npm install --save-dev reify
  echo '.reify-cache/' >> .gitignore
  alias knex='node -r reify ./node_modules/.bin/knex

  knex migrate:latest # …etc

*/

const table = 'foo';

export const up = knex => {
  return knex.schema.createTable(table, t => {
    t.increments();
    // …etc
    t.timestamps();
  });
};

export const down = knex => knex.schema.dropTable(table);

// OR

// export function up (knex) {
//   return knex.schema.createTable(table, t => {
//     t.increments();
//     // …etc
//     t.timestamps();
//   });
// }

// export function down (knex) {
//   knex.schema.dropTable(table);
// }

Any es6ifying knexfile.js attempts are ugly but I can live with a single anomaly using module.exports

@haywirez
Copy link

Keep getting No knexfile found in this directory. Specify a path with --knexfile with the following:

// src/knexfile.js
import config from 'config'

export default {
  debug: false,
  settings: config.get('dbConfig'),
  seeds: {
    directory: './seeds/'
  }
}
//package.json
 "scripts": {
    "knex": "babel-node node_modules/.bin/knex"
  }
yarn knex migrate:make lol

Any pointers? Seems to be just a working dir issue (I guess) 🤔

@kibertoad
Copy link
Collaborator

@haywirez Are you sure this is related to the topic of this ticket?.. Anyway, try doing exactly what it is asking, add --knexfile that would point to your knexfile.js

@mAAdhaTTah
Copy link

It's possible @haywirez' issue is because export default { ... } will need to be const file = require('/path/to/knexfile').default for ESM interop.

Also, Liftoff, as used by the knex cli, allows for a --require flag for requiring something like esm or babel-register. Unfortunately, because this command isn't registered in Commander, the knex cli dies after requiring in esm:

± |master ?:18 ✗| → npx knex --require esm seed:make admin_user
Requiring external module esm

  error: unknown option `--require'

Would the project be open to a PR to enable --require on all commands as well as ensure interop on the knexfile?

@gordysc
Copy link
Contributor

gordysc commented Aug 18, 2018

#1232 (comment) would also allow you to register tsconfg-paths for alias path names, which is useful when making seeds w/ typescript. ts-node doesn't plan to support alias paths so this becomes problematic when I'm making seeds that require some complex transformations before persisting data to the database. If we exposed the --require flag I can get around this myself.

Alternatively, if others hit this issue, you can get around it without the --require flag by doing:

node -r tsconfig-paths/register node_modules/.bin/knex seed:run

@kibertoad
Copy link
Collaborator

@olalonde Is this still relevant in a world where all popular Node.js versions support ES6 out-of-the-box?

@mAAdhaTTah
Copy link

@kibertoad Node doesn't support ESModules yet.

@kibertoad
Copy link
Collaborator

@mAAdhaTTah It sorta kinda does: https://nodejs.org/api/esm.html

@mAAdhaTTah
Copy link

@kibertoad Yeah, but it's very experimental & unstable, so you can't rely on it.

@codingpop
Copy link

codingpop commented Nov 11, 2018

If you're using esm, on Windows, try this:
node -r esm node_modules/knex/bin/cli.js migrate:make migration_name.

On Linux and Mac, try
node -r esm node_modules/.bin/knex migrate:make migration_name

@machineghost
Copy link
Contributor

machineghost commented Jun 14, 2019

So the last update I see on this ticket involves esm and is over half a year old ... has anyone managed to get this working with Babel since? I'm particularly curious with all the changes to Babel, and the library's seeming new preference for using @babel/register instead of babel-node.

@kibertoad
Copy link
Collaborator

@machineghost You can try latest master, it doesn't use any transpilation, so might work better. If you run into any remaining limitations, please let me know.

@machineghost
Copy link
Contributor

machineghost commented Jun 14, 2019

It still doesn't seem to work. My package.json is the following:

"knex": "npx @babel/node node_modules/.bin/knex --knexfile=src/database/knexfile.js",
"migrate": "npm run knex migrate:latest",

// ...
"dependencies": {
// ...
"knex": "git://github.com/tgriesser/knex.git#master",
// ...

and my knexfile.js is super simple:

import { development } from './realKnexFile';
module.exports = {
    development
};

But it can't get past the first line. When I run npm run migrate I get:

knexfile.js:1
import { development } from './realKnexFile';
       ^

SyntaxError: Unexpected token {
    at Module._compile (internal/modules/cjs/loader.js:760:23)

@machineghost
Copy link
Contributor

machineghost commented Sep 16, 2019

Having discovered the amazing "esm" module (from the creator of Lodash), one doesn't even need Babel to be involved anymore. Really all Knex needs to support ES6 modules, super easily, is just a way to pass --require esm to node behind the scenes.

If the knex command line could simply take -r and pass it to Node, it would not only solve this ticket but also any other issue where someone needs to run a Node module before running Knex.

-EDIT-

Until a -r argument exists, one can use ES6 syntax in all required modules (but not in knexfile.js itself) by doing:

require = require("esm")(module);
const importedContent = require('./someFile.js');

This will make require itself handle ES6 modules just fine ... but the knexfile.js still has to use require and module.exports; a -r option would solve that.

@orlando
Copy link

orlando commented Oct 2, 2019

In case someone is looking, here's a full example on how to use esm to work with knex

knexfile.js

const config = {
}

// Knex will fail if you use "exports default"
module.exports = config

package.json

{
  "scripts": {
    "knex": "node -r esm node_modules/.bin/knex",
    "db:migrate": "yarn knex migrate:latest"
  }
}

@jhechtf
Copy link

jhechtf commented Mar 9, 2020

With ESM becoming more popular I would be willing to look into making some contribution to allow for Knex to work with Node >12. I have a bit of experience working with ESM in Node, so hopefully this goes smoothly. Unfortunately I can't tell you when I would be able to get started on this as I can't work on it during work hours (not business related). It's also going to take me a minute to get accustomed to the knex codebase

@machineghost
Copy link
Contributor

I think it might be as simple as adding require('esm') as the new line one ... if the library's authors are ok with the new dependency.

@kibertoad
Copy link
Collaborator

@jhechtf @machineghost Check this out:
#3616
#3639
#3638

Also Liftoff support was fixed, so before doing more changes it would be helpful to write some tests to figure out what works and what does not now.
That said, if someone investigates all this and decides what work (if any) is still needed and contributes some tests, that would be super helpful.

@machineghost
Copy link
Contributor

machineghost commented Mar 9, 2020

Yeah, you guys went with the official Node solution, which isn't fully baked yet.

IMHO the "esm" module solution is much simpler, and wouldn't require any special flag: it would just make the library ES Module friendly for everyone, in all Node environs (very likely with a just single require line, as I said).

It's from the guy who made Lodash; JDD rocks.

@kibertoad
Copy link
Collaborator

@machineghost #3639 uses babel, so that's one other way. I thnk I saw someone trying the esm module approach, but for some reason can't find the PR now.

@machineghost
Copy link
Contributor

machineghost commented Mar 9, 2020

Well, FWIW that library is my current fastest source of Stack Overflow points ;)

I discovered it and recommended it to others here: https://stackoverflow.com/questions/47277887/node-experimental-modules-requested-module-does-not-provide-an-export-named/54302557#54302557

and now I get a new upvote every few days because it truly is a great (arguably the best, right now) solution for ESM.

@kibertoad
Copy link
Collaborator

@machineghost How intrusive would you expect its introduction to be based on existing codebase?

@machineghost
Copy link
Contributor

machineghost commented Mar 9, 2020

I think this is one of these "it should have no effect whatsoever, but since it affects everything you have to test to be sure" deals.

I haven't looked at its internals, but given the profile of this project in the community, perhaps if someone asked @jdalton he might weigh in on the risks (with a much more informed opinion than mine?) 🤞

@kibertoad
Copy link
Collaborator

@machineghost You can take a look at #3616 to see what was the impact of original implementation. Probably that would have to be removed or replaced if esm is introduced.

@machineghost
Copy link
Contributor

I don't have bandwidth ATM, but I'll take a look when I have time. Perhaps @jhechtf might be able to sooner?

@briandamaged
Copy link
Collaborator

Yeah, you guys went with the official Node solution, which isn't fully baked yet.

@machineghost : Hmm... I believe the --esm flag is actually using the esm module you mentioned:

knex/bin/cli.js

Lines 52 to 55 in 0f523db

if (opts.esm) {
// enable esm interop via 'esm' module
require = require('esm')(module);
}

On a related note: you just reminded me that I meant to fix the --require flag after the Liftoff enhancements had been merged. I should go do that now...

@machineghost
Copy link
Contributor

machineghost commented Mar 10, 2020

Oh, my apologies! I was scanning too quickly, saw "node ${KNEX} --esm", and just looked past the variable there and thought you were passing an arg to Node directly to tell it to use Node's version.

It sounds like this has already been solved, and that just wasn't clear from the issue thread; nice job 👍

@briandamaged
Copy link
Collaborator

np! It looks like the feature was added by @D10221 in this PR:

#3616

So, they deserve the credit 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests