Issue with the custom state storage example

[johncmunson]

I needed the ability to store the state of my migrations in the database itself. I noticed that there was an example for doing so. Luckily, I was already using postgres as my database just like in the example, so I realized I could use the example code pretty much as-is.

To get it to work though, I needed to add in some extra error handling to account for the first time migrations ever get run. It appears that there was already an attempt to account for this, but I don't believe it was enough. Please see below for my modifications.

load: async function (fn) {
    await pg.connect()

    // Load the single row of migration data from the database
    let rows
    try {
      rows = (await pg.query('SELECT data FROM migrations')).rows
    } catch (error) {
      console.log('No migrations found.')
    }

    if (!rows || rows.length !== 1) {
      console.log('Cannot read migrations from database. If this is the first time you run migrations, then this is normal.')
      return fn(null, {})
    }

    // Call callback with new migration data object
    await pg.end()
    fn(null, rows[0].data)
  },

[johncmunson]

@tj if you concur, I'd be happy to create a pull request with the changes

[johncmunson]

Also, I was able to successfully run the script programmatically, i.e. node custom-state-storage.js, but it's not clear to me how I would run it from the command line like your README describes... migrate up --store="./custom-state-storage.js".

Here is my modified code to attempt to run it from the command line...

'use strict'
const migrate = require('migrate')
const {Client} = require('pg')
const db = require('./db')

/**
 * Stores and loads the executed migrations in the database. The table
 * migrations is only one row and stores a JSON of the data that the
 * migrate package uses to know which migrations have been executed.
 */
const customStateStorage = {
  load: async function (fn) {
    // Load the single row of migration data from the database
    let rows
    try {
      rows = (await db.query('SELECT data FROM migrations')).rows
    } catch (error) {
      console.log('No migrations found.')
    }

    if (!rows || rows.length !== 1) {
      console.log('Cannot read migrations from database. If this is the first time you run migrations, then this is normal.')
      return fn(null, {})
    }

    // Call callback with new migration data object
    fn(null, rows[0].data)
  },

  save: async function (set, fn) {
    // Check if table 'migrations' exists and if not, create it.
    await db.query('CREATE TABLE IF NOT EXISTS migrations (id integer PRIMARY KEY, data jsonb NOT NULL)')

    await db.query(`
      INSERT INTO migrations (id, data)
      VALUES (1, $1)
      ON CONFLICT (id) DO UPDATE SET data = $1
    `, [{
      lastRun: set.lastRun,
      migrations: set.migrations
    }])

    fn()
  }
}

module.exports = customStateStorage

And this is the stack trace that it errors out with...

/usr/src/app/node_modules/migrate/bin/migrate-up:64
var store = new Store(program.stateFile)
            ^

TypeError: Store is not a constructor
    at Object.<anonymous> (/usr/src/app/node_modules/migrate/bin/migrate-up:64:13)
    at Module._compile (internal/modules/cjs/loader.js:805:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:816:10)
    at Module.load (internal/modules/cjs/loader.js:672:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:612:12)
    at Function.Module._load (internal/modules/cjs/loader.js:604:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:868:12)
    at internal/main/run_main_module.js:21:11

[wesleytodd]

Hi @johncmunson, I would happily accept more robust example code as a PR. For your second issue, I think this might not have been the best implementation in hindsight, but the Store is required to be a constructor. So it looks like you are exporting a plain object with load/save. Try exporting a function/class with those methods and it should work.

[wesleytodd]

Oh, and checking out the example I can see that it is broken in exactly the way I described above. That would be a great part of the PR you can open to fix that.

[johncmunson]

@wesleytodd thank you for the feedback. I'll try to find some time later this week to clean up the example and submit a PR.

[wesleytodd]

@johncmunson did you ever have time to take a look at this?

[krishna-atkalikar]

I ran into same problem of Store is not a constructor, stacktrace below:

var store = new Store(program.stateFile)
            ^

TypeError: Store is not a constructor
    at Object.<anonymous> (/usr/local/lib/node_modules/migrate/bin/migrate-up:64:13)
    at Module._compile (internal/modules/cjs/loader.js:777:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:788:10)
    at Module.load (internal/modules/cjs/loader.js:643:32)
    at Function.Module._load (internal/modules/cjs/loader.js:556:12)
    at Function.Module.runMain (internal/modules/cjs/loader.js:840:10)
    at internal/main/run_main_module.js:17:11

What is the solution to this problem? I have used the code from examples directory to change it to mongodb.

[wesleytodd]

@krishna-atkalikar, the answer is above:

the Store is required to be a constructor. So it looks like you are exporting a plain object with load/save. Try exporting a function/class with those methods and it should work.

It is just the example which is wrong. I would be happy to accept a PR fixing the example.

[krishna-atkalikar]

@wesleytodd created PR https://github.com/tj/node-migrate/pull/162

[wesleytodd]

@krishna-atkalikar, it looks like the PR you opened is a new example. Which is great! But just so others who might land here know, the existing example with pg is still wrong, and a PR to fix that would still be great.

[krishna-atkalikar]

@wesleytodd I'll try to fix the issue with pg example.

[ceciliazcx]

@johncmunson @krishna-atkalikar Hi here, I got the exactly same problem as you two. Could I know any luck you got this issue fixed?

[wesleytodd]

Hi @ceciliazcx, the discussion above has your answer. The example is wrong and the object is not a constructor. Feel free to submit a PR which changes it.

[ceciliazcx]

@wesleytodd Thanks for your reply. I am not sure we are at same page. in my own code, I do have constructor inside store. but I still get the error. If you know how to fix the error. Do you mind simply let me know? I have stuck here for a while.

export class Store {
  constructor () { }
  async load (fn: Function) {
    ...
  }
  async save (set: any, fn: Function) {
   ...
  }
}

const migrate = require('migrate');
migrate.load({
  stateStore: new Store(),
}, (err:any, set:any) => {
...
});

[wesleytodd]

EDIT: I was going off the previous posts error message and your code. I was wrong in my OP.

You need to post more than this for me to be able to help. The code is fine, but also please post the error message and stack trace you have.

[msanguineti]

@ceciliazcx if you are using typescript, do this:

// my-store.ts
class Store {
  // no constructor declaration
  ...
  async load ...
  async save ...
}

export = Store // export NOT exports

Then you can have a file somewhere to call programmatically migrate with your store

// migration-runner.ts
import * as migrate from 'migrate'
import Store = require('./my-store')

migrate.load({
  stateStore: new Store(),
}, (err:any, set:any) => {
...
})

If you keep the store and the runner separated you can then also use the store from the cli with a ts compiler (or use the compiled one :))

[rjhilgefort]

@msanguineti Thanks for the TS tip, it put me on the right path and saved me some time with that export! I have some TS types for this lib that I'll try to remember to PR into definitely typed at some point if you're interested.

[wesleytodd]

Hey @rjhilgefort, you might consider PRing them here. I would be happy to put together a release which contained the types if it had some tests around their validity. That way we can own them and not rely on DT updating them as we make changes.

[rjhilgefort]

Sounds good! They're very much still a WIP, but I'll try to formalize them into a PR once I finish implementing the migration work for the project I'm working on. In the mean time, here's what I have so far- I'm building as I go:

migrate.d.ts

declare module 'migrate' {
  export type UnixTimestamp = number
  export type MigrationFileName = string

  export type Migration = {
    title: MigrationFileName
    description: string
    timestamp: UnixTimestamp
  }

  export type MigrateState = {
    lastRun: MigrationFileName
    migrations: Array<Migration>
    store: {} // This is `{}` in initial testing, not sure what is stored there.
    map: Record<MigrationFileName, Migration>
  }

  export type LoadCb = (options: {} | null, migrateState: MigrateState) => void

  export type SaveCb = () => void

  export interface CustomStateStore {
    init(): Promise<void>
    load(fn: LoadCb): Promise<void>
    save(set: MigrateState, fn: SaveCb): Promise<void>
  }
}