knex: Ignore duplicate variants of same migration (e.g. `.js`/`.ts`)

Environment

Knex version: 0.15.2 Database + version: any OS: any

Feature discussion / request

Use case

Code that implements migrations in TypeScript (or CoffeeScript or some other dialect that gets compiled to plain JavaScript) needs to be aware of different filename extensions in development VS production.

Even with the loadExtensions setting introduced in #2168, it is sometimes difficult to tell exactly when to use (only) the .ts file and when to use (only) the .js file.

One particular issue that I experience is this:

During development, I use ts-node to run Knex migrations. I want that to use the .ts files.
During testing, I rely on Jest and Wallaby.js. Jest sees only the .ts files. Wallaby.js sees the .ts files and .js files created just-in-time directly next to them. I want both to use the .ts files only.
During production, I only have the .js files available in the migrations directory. Nonetheless, I want to be able to operate on the same database instance that I used during development, i.e. seamlessly run the dev version or the prod version of my application on any database.
Ideally, I would use the same Knex configuration and not adjust more settings than necessary between dev and prod to make tests more reliable.

Requested feature

Use loadExtensions to only load any given basename once, with the extension that first appears in the loadExtensions list.

Example: Suppose the migrations directory contains the following files:

001.ts (A, original TypeScript), 001.js (A, built file)
002.ts (B, original TypeScript)
003.js (C, built file)

We have three migrations (001, 002, 003). When loadExtensions is set to [".ts", ".js"], Knex should only consider the following files to apply as migrations instead of all four files as is the case now:

001.ts (.ts has priority over .js as it is listed before the latter in loadExtensions)
002.ts (there is no other file for the 002 basename)
003.js (ditto)

API / Implementation proposal

Function listAll() in src/migrate/migration-list-resolver.js could be adjusted to only return the first instance of each migration with the same basename (i.e. migrations that only differ in their extension), with priority of extension given by their order in loadExtensions as described above.

In addition, it would be necessary to replace calls to lodash’s difference() with differenceWith() so that extensions get ignored when comparing all available migrations to the ones that have already been applied and stored in the database (returned by listAll() and listCompleted(), respectively).

Alternatively, it would be possible to only ever operate on the basenames of each migration file and store only that in the database (i.e. a migration would only refer to the basename instead of the complete filename). This would, however, be a breaking change and probably require a flag in the config to activate this feature, though it would arguably be the cleaner way of implementing the suggested feature.

Maybe it would be possible to implement the suggested changes as a follow-up to the support for multiple directories in #2690 (#2735).

Possible side-effects

Migrations that share the same basename but have different extensions would be seen as a single migration when this proposal gets implemented. Previously, every file in the migrations directory would be considered a migration. If one had deliberately chosen two migrations to have the same basename and differ only in their extension, only one of them would get applied with this proposal implemented.

However, since it is bad practice to have two different modules with the same basename and different extensions, this should not be a typical use case. This proposal is therefore not expected to break any existing migrations.

NB: All things said in this issue would apply to both migrations as well as seed files.

About this issue

Original URL
State: open
Created 6 years ago
Reactions: 8
Comments: 24 (12 by maintainers)

Most upvoted comments

I’ll pick this task up after https://github.com/tgriesser/knex/pull/2775 is merged, otherwise I might introduce some conflicts.

kibertoad on Aug 30, 2018

We have similar issues here, and we were thinking that it would actually be enough for us if it were possible to ignore the extension (omitting it when storing the filename in the database or ignoring it when comparing the list of migrations).

flipace on Aug 30, 2018

@sgoll not sure if you had a look at my links, but this functionality exists already inside knex @next.

     class MyMigrationSource {
        // Must return a Promise containing a list of migrations. 
        // Migrations can be whatever you want, they will be passed as
        // arguments to getMigrationName and getMigration
        getMigrations() {
          // In this example we are just returning migration names
          return Promise.resolve(['migration1'])
        }
        getMigrationName(migration) {
          return migration;
        }
        getMigration(migration) {
          switch(migration) {
            case 'migration1':
              return {
                up(knex)   { /* ... * / }
                down(knex) { /* ... * / }
              }
          }
        }
      }

      // pass an instance of your migration source as knex config
      knex.migrations.latest({ migrationSource: new MyMigrationSource() })

JakeGinnivan on Oct 4, 2018

@elhigu

But sholdnt it work other way around, that compiled/.gitignore files are actually ran doesnt make much sense to compile them if they will never be ran…

Jest uses the raw .ts files for running tests. This is particularly useful when running Jest in --watch mode for test-driven development.

sgoll on Oct 3, 2018

I’m fine with doing this as breaking change and just release it in 0.16. I would be surprised if anyone has been using knex and been running many migrations with same name, but different extensions.one thing to consider there is that we might not like to store migration extension anymore in database or we should ignore it when comparing migration filename list.

elhigu on Aug 18, 2018