node-pg-migrate icon indicating copy to clipboard operation
node-pg-migrate copied to clipboard

Support explicitly listing migrations, instead of using a filesystem loading function

Open moltar opened this issue 4 years ago • 8 comments

I am bundling a bunch of code using ~~webpack~~ esbuild to deploy as a lambda function. One of the functions does the migrations. However, because all of the code is bundled as a single file, the result is that node-pg-migrate cannot find the migration files.

E.g. TypeORM supports explicit array of migration classes.

Thanks.

moltar avatar Jun 16 '20 10:06 moltar

Would you accept a PR for this?

moltar avatar Jul 03 '20 14:07 moltar

Yes, I will review PR if you create one

dolezel avatar Jul 07 '20 06:07 dolezel

@dolezel this doesn't seem to be too difficult to achieve, but I have one blocker that I want to get you to check on / approve.

Migration class has a migrationPath parameter.

I can see it is used for generating the name of the migration:

this.name = path.basename(migrationPath, path.extname(migrationPath))

This can be worked around by checking the class name, or explicitly providing the migration name (like in TypeORM).

The other use case I found was is in the return value of the main entrypoint (runner) function:

    return toRun.map((m) => ({
      path: m.path,
      name: m.name,
      timestamp: m.timestamp,
    }))

And for this one I don't know if this is being used anywhere, as this can be used by external tools or consuming projects.

Do you think it would be safe to remove the migrationPath parameter requirement for Migration class? We can generate the name externally, and pass the name, rather than the path itself.

Thanks.

moltar avatar Jul 08 '20 02:07 moltar

I think that full name generation can be extracted to function and class can be provided this full name. I'm not sure if the output of runner is used, but I would assume it is. But it should not be difficult to split full name into path and basename.

dolezel avatar Jul 08 '20 05:07 dolezel

Well, we'd have to drop the path completely as it wouldn't be available when using classes directly.

We can use the path for generating names for the file-based migrations.

But when a class is provided directly, there would be no path.

Just to be clear, this is what I am talking about:

// Migration class
class SomeMigration0123456789 implements MigrationBuilderActions {
  // add a name prop, but it can default to class name (SomeMigration0123456789)
  name = 'my migration'

  up(pgm: MigrationBuilder) {
    // ...
  }

  down(pgm: MigrationBuilder) {
    // ...
  }
}

// runner
migrate({
  databaseUrl: '...',
  direction: 'up',
  // pass an array of migration classes to the runner
  migrations: [
    SomeMigration0123456789
  ]
})

moltar avatar Jul 09 '20 09:07 moltar

Yeah, I think there should be an interface/class for Migration just for up and down methods (and maybe few other things) and also there will be e.g. FileMigration which can extend that class (or maybe have a builder which will read and create it from file definition)

dolezel avatar Jul 10 '20 09:07 dolezel

This would be really handy when used with something like pkg or ncc where accessing files is a pain.

philjones88 avatar Aug 25 '20 14:08 philjones88