feat: added support for database migrations #91
Conversation
yann510
force-pushed
the
22-migration-support
branch
2 times, most recently
from
7947ae4
to
915afb5
Compare
yann510
force-pushed
the
22-migration-support
branch
from
915afb5
to
bca84ab
Compare
apps/database/postgres/ormconfig.ts
Outdated
| password: process.env.DATABASE_PASSWORD || "postgres", | ||
| database: process.env.DATABASE_NAME || "stator", | ||
|
|
||
| entities: [__dirname + "/../../../**/*.entity{.ts,.js}"], |
There was a problem hiding this comment.
use @stator/libs/models here instead of long path
There was a problem hiding this comment.
It's not supported by the compiler option I need to set for the migrations to work. :(
apps/database/postgres/ormconfig.ts
Outdated
| @@ -0,0 +1,18 @@ | |||
| import { ConnectionOptions } from "typeorm" | |||
|
|
|||
| const config: ConnectionOptions = { | |||
There was a problem hiding this comment.
you added this but you're not using it in the app? also where is synchronize?
There was a problem hiding this comment.
So this file is used only when running the migration which it's path is specified in the npm script command. Locally migrations will be done automatically as they will be synchronized from the model. I'm not sure if that's the right approach though. Maybe we should have the same process locally as in production? We could run the migrations on API startup to ensure we're always up to date.
What do you think?
There was a problem hiding this comment.
We could. But my main concern is that we are duplicating the config here and in the api. We should use the same for both to avoid modification errors
There was a problem hiding this comment.
That is a very valid concern.
I had tried importing the configuration file from the API, but since I need to target commonJS, it doesn't support the paths provided in the tsconfig.base.json. One way this could work would be to flip things around where ormconfig.ts would be the source of truth, and the API configuration file would import the ormconfig.ts file properties.
Makes sense?
There was a problem hiding this comment.
Yep, I agree with the swap
Pull Request Test Coverage Report for Build 340441895
💛 - Coveralls |
yann510
force-pushed
the
22-migration-support
branch
3 times, most recently
from
4cad857
to
bd67ea9
Compare
yann510
force-pushed
the
22-migration-support
branch
from
bd67ea9
to
616079c
Compare
README.md
Outdated
| @@ -211,6 +211,28 @@ To ensure your developers don't get into any trouble while installing those, the | |||
| **By default, the project uses postgres.** | |||
| If this is what you want, you're good to go; everything will work out of the box. | |||
|
|
|||
| #### Migrations | |||
|
|
|||
| By default, the automatic synchronization is activate between your models and the database. | |||
There was a problem hiding this comment.
| By default, the automatic synchronization is activate between your models and the database. | |
| By default, the automatic synchronization is activated between your models and the database. |
README.md
Outdated
| ``` | ||
| npm run typeorm -- migration:generate -n {MIGRATION_NAME} | ||
| ``` | ||
| This will check the difference of models between your defined entities your database schemas. |
There was a problem hiding this comment.
| This will check the difference of models between your defined entities your database schemas. | |
| This will check the difference between models for your defined entities and your database schemas. |
apps/database/orm-config.ts
Outdated
| username: process.env.DATABASE_USERNAME || "postgres", | ||
| password: process.env.DATABASE_PASSWORD || "postgres", | ||
| database: process.env.DATABASE_NAME || "stator", | ||
| synchronize: !!process.env.DATABASE_SYNCHRONIZE || true, |
There was a problem hiding this comment.
this is never going to work. if you set it to false, it will fallback to true. you should use the ?? operator instead
yann510
force-pushed
the
22-migration-support
branch
from
7ddcc79
to
4ea7b38
Compare
|
🎉 This PR is included in version 1.11.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
No description provided.