lukeed / ley

(WIP) Driver-agnostic database migrations
MIT License
260 stars 14 forks source link

Allow async config file exports #9

Closed TehShrike closed 4 years ago

TehShrike commented 4 years ago

This would allow config files to be CommonJS files that pull in connection details from ESM files using dynamic imports.

Alternately, util.local could be changed to be async, and use dynamic imports if the file extension is .mjs or something, but forcing config files to be CJS while allowing asynchrony seemed less problematic.

codecov-commenter commented 4 years ago

Codecov Report

Merging #9 into master will decrease coverage by 2.13%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #9      +/-   ##
==========================================
- Coverage   48.07%   45.94%   -2.14%     
==========================================
  Files           2        2              
  Lines         104      111       +7     
==========================================
+ Hits           50       51       +1     
- Misses         54       60       +6     
Impacted Files Coverage Δ
index.js 11.76% <0.00%> (+0.28%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c97dc45...6c37920. Read the comment docs.

lukeed commented 4 years ago

I think I'm good with this. Just to make sure we're on the same page, can you share a demo config file that'd use this?

lukeed commented 4 years ago

Accident? 🤔

TehShrike commented 4 years ago

oh shit, yeah, apparently I accidentally wiped this out while going push-crazy on the command-line

TehShrike commented 4 years ago

Here's a config file I'm imagining:

module.exports = import('./environment.mjs').then(({ db_user, db_password }) => ({
    host: 'localhost',
    database: 'my_sweet_app',
    user: db_user,
    pass: db_password
}))

I tested this change locally by just using Promise.resolve e.g.

module.exports = Promise.resolve({
    host: 'localhost',
    database: 'my_sweet_app',
    user: 'testuser',
    pass: 'somepass'
})
lukeed commented 4 years ago

I don't really know why but those examples give me the heebie jeebies haha. It feels a bit off to do module.exports = Promise, even though module.exports = async function () {} is exactly the same thing and feels fine to me. 🤷

TehShrike commented 4 years ago

hah, yeah, I can understand that. Taste could easily drive you to

module.exports = async () => {
    const { db_user, db_password } = await import('./environment.mjs')

    return {
        host: 'localhost',
        database: 'my_sweet_app',
        user: db_user,
        pass: db_password
    }
}

I put the await on the outside because I figured allowing either version was the reasonable thing to do.