adrien2p / medusa-extender

:syringe: Medusa on steroid, take your medusa project to the next level with some badass features :rocket:
https://adrien2p.github.io/medusa-extender/
MIT License
322 stars 39 forks source link

Generate Migration TypeORM #113

Open juanzgc opened 2 years ago

juanzgc commented 2 years ago

Describe the bug Generating a migration results in errors if the Entity that you are overriding has column types of other Entities (that are imported).

TypeORMError: Entity metadata for Product#images was not found. Check if you specified a correct entity object and if it's connected in the connection options.
    at new TypeORMError (/Users/juan/renlo/node_modules/src/error/TypeORMError.ts:7:9)
    at /Users/juan/renlo/node_modules/src/metadata-builder/EntityMetadataBuilder.ts:687:23
    at Array.forEach (<anonymous>)
    at EntityMetadataBuilder.computeInverseProperties (/Users/juan/renlo/node_modules/src/metadata-builder/EntityMetadataBuilder.ts:682:34)
    at /Users/juan/renlo/node_modules/src/metadata-builder/EntityMetadataBuilder.ts:119:56
    at Array.forEach (<anonymous>)
    at EntityMetadataBuilder.build (/Users/juan/renlo/node_modules/src/metadata-builder/EntityMetadataBuilder.ts:119:25)
    at ConnectionMetadataBuilder.<anonymous> (/Users/juan/renlo/node_modules/src/connection/ConnectionMetadataBuilder.ts:65:111)
    at step (/Users/juan/renlo/node_modules/tslib/tslib.js:144:27)
    at Object.next (/Users/juan/renlo/node_modules/tslib/tslib.js:125:57)
error Command failed with exit code 1.

For example:

Whereas User doesn't face this error because there are no column types of other imported entities.

It seems that for whatever reason, when generating the migration with extended Entities, typeorm has difficulty with nested imports.

@medusajs/medusa/dist/models/product.d.ts:

image

@medusajs/medusa/dist/models/user.d.ts:

image

To Reproduce Steps to reproduce the behavior:

  1. Create a ormconfig.json
    {
    "type": "postgres",
    "url": "<INSERT>",
    "synchronize": false,
    "entities": ["dist/modules/**/*.entity.js"],
    "migrations": ["dist/modules/migrations/*.js"],
    "cli": {
    "migrationsDir": "src/modules/migrations"
    }
    }
  2. Add the following scripts in package.json:
    "typeorm": "node --require ts-node/register ./node_modules/typeorm/cli.js",
    "migration:generate": "yarn typeorm migration:generate",
  3. Install typeorm:
yarn add typeorm@0.2.45
  1. Extend the Product Entity
import { Column, Entity, Index } from 'typeorm';
import { Product as MedusaProduct } from '@medusajs/medusa/dist/models';
import { Entity as MedusaEntity } from 'medusa-extender';

@MedusaEntity({ override: MedusaProduct })
@Entity()
export class Product extends MedusaProduct {
  @Index()
  @Column({ nullable: false })
  store_id: string;
}
  1. Generate Migration:
    yarn build
    yarn typeorm migration:generate -n InitialProductMigration
  2. You will see the error Entity metadata for Product#images was not found. However, if you add an export to the Product Entity extender you won't see the same error. The error will now be Entity metadata for Product#options was not found.
    
    import { Column, Entity, Index } from 'typeorm';
    import { Product as MedusaProduct } from '@medusajs/medusa/dist/models';
    import { Entity as MedusaEntity } from 'medusa-extender';

export { Image } from '@medusajs/medusa/dist/models';

@MedusaEntity({ override: MedusaProduct }) @Entity() export class Product extends MedusaProduct { @Index() @Column({ nullable: false }) store_id: string; }


The line changed was: `export { Image } from '@medusajs/medusa/dist/models';`

Don't forget to `yarn build` before re-attempting to generate the migration.

TypeORMError: Entity metadata for Product#options was not found. Check if you specified a correct entity object and if it's connected in the connection options.



You can continue adding all the remaining exports until the migration is generated, however, it will also generate a **_faulty_** migration because it will include multiple FKs and Indexes that it attempts to delete. I believe if we could somehow move all Entities into one file and export from one file (at build time), we would no longer see these migration errors.

**You can also try this with an easier Entity such as `Store` which has less Columns that are referenced by other Entities**

**Expected behavior**
Generate a migration without having to go through these hacks to get one generated. Generating migrations in the Medusa Core doesn't have these issues.

**Screenshots**
If applicable, add screenshots to help explain your problem.

**Package version:**
 - Version 1.7.4

**Additional context**
[Similar Issue](https://github.com/typeorm/typeorm/issues/420)
juanzgc commented 2 years ago

With that being said, i'm not quite sure of the fix to this issue. We should be able to use TypeORM's Generate Migration capabilities considering it is an ORM, and we are already doing this in Core MedusaJS.

But our issue here is with importing the Models from Medusa Core into the Extender and still having the same capabilities of generating a migration.

adrien2p commented 2 years ago

I've just tried to reproduce, and I did not get any error. here is a screen shot of the output

Screenshot 2022-08-14 at 11 33 32
adrien2p commented 2 years ago

As discussed, it seams to come from typeorm driver postgres during changes check. So it seams to be linked to typeorm not managing properly the checks

https://github.com/medusajs/medusa/issues/2054#issuecomment-1217660882

juanzgc commented 2 years ago

Adding a few more details per our discussion:

This will require updating to TypeORM v3 which includes breaking changes that will need to be addressed beforehand. Changes will include and are not limited to: connection, repositories, queries, options, etc...

At the moment it seems that TypeORM can't distinguish between entities from different repositories which results in the issue that we're seeing.

aboozaid commented 2 years ago

I fixed this issue by using .env and add these lines TYPEORM_ENTITIES=./node_modules/@medusajs/medusa/dist/models/*.js,./dist/models/*.js TYPEORM_MIGRATIONS=./node_modules/@medusajs/medusa/dist/migrations/*.js,./dist/migrations/*.js typeorm is unable to find medusa core models that's why giving you the error above and keep sure you use medusa ver 1.3.5

adrien2p commented 2 years ago

@juanzgc did you give it a shot 👆

juanzgc commented 2 years ago

I have not, will attempt this later tonight 👀

ZiaSoltani2023 commented 2 years ago

@aboozaid where did you put your .env file in medusa-store? can you please give me a path? thanks

juanzgc commented 2 years ago

@aboozaid I've added the following in the .env (my folder structure is a bit different):

TYPEORM_ENTITIES=./node_modules/@medusajs/medusa/dist/models/*.js,./dist/modules/**/*.entity.js
TYPEORM_MIGRATIONS=./node_modules/@medusajs/medusa/dist/migrations/*.js,./dist/modules/migrations/*.js

however, I'm still seeing the same errors. I think it's worth mentioning, I can add the following in ormconfig.js:

  "entities": [
    "./src/modules/**/*{.js,.ts}",
    "node_modules/@medusajs/medusa/dist/models/**/*{.js,.ts}"
  ],
  "migrations": [
    "./src/migrations/**/*.migration{.js,.ts}",
    "node_modules/@medusajs/medusa/dist/migrations/**/*{.js,.ts}"
  ],

and even though migrations will now always generate, they won't generate correctly. It will attempt to drop indices and foreign keys that shouldn't be dropped.

Are you using a ormconfig.js file? What is the difference between adding those values via a .env file vs ormconfig.js file? If you don't mind, would it be possible to provide a more detailed solution?

I'm not sure if @adrien2p, was able to also give it a shot. But personally, it didn't seem to work on my end. cc @adrien2p

aboozaid commented 2 years ago

@aboozaid where did you put your .env file in medusa-store? can you please give me a path? thanks

I have a monorepo project so I put it in the root

@aboozaid I've added the following in the .env (my folder structure is a bit different):

TYPEORM_ENTITIES=./node_modules/@medusajs/medusa/dist/models/*.js,./dist/modules/**/*.entity.js
TYPEORM_MIGRATIONS=./node_modules/@medusajs/medusa/dist/migrations/*.js,./dist/modules/migrations/*.js

however, I'm still seeing the same errors. I think it's worth mentioning, I can add the following in ormconfig.js:

  "entities": [
    "./src/modules/**/*{.js,.ts}",
    "node_modules/@medusajs/medusa/dist/models/**/*{.js,.ts}"
  ],
  "migrations": [
    "./src/migrations/**/*.migration{.js,.ts}",
    "node_modules/@medusajs/medusa/dist/migrations/**/*{.js,.ts}"
  ],

and even though migrations will now always generate, they won't generate correctly. It will attempt to drop indices and foreign keys that shouldn't be dropped.

Are you using a ormconfig.js file? What is the difference between adding those values via a .env file vs ormconfig.js file? If you don't mind, would it be possible to provide a more detailed solution?

I'm not sure if @adrien2p, was able to also give it a shot. But personally, it didn't seem to work on my end. cc @adrien2p

Add these to .env file

TYPEORM_CONNECTION=postgres
TYPEORM_URL=postgres://localhost/your_database_name

when I migrate or create migrations I always use typeorm commands without any ormconfig.js file as I put all configs in .env file and typeorm automatically read from it

juanzgc commented 2 years ago

I can confirm this doesn't work on my end. The migration itself will generate but it will be full of constraint removals that are completely incorrect and not what we expect.

As expected, there isn't a difference between ormconfig.js and using environment variables. Atleast from what I've tested locally.

@aboozaid Would it be possible to see a live demo of you generating migrations? Could you try extending the Store entity. Some entities, for example User which don't have any referencing entities will actually generate without a problem. The problem begins with entities that do have references. Would be good to possibly get a live demo if you are available.

juanzgc commented 2 years ago

It seems that in Typeorm 3 - this solution might work but we will have to wait until Medusa is migrated to use Typeorm v3: https://github.com/typeorm/typeorm/issues/9122#issuecomment-1160347397

ZiaSoltani2023 commented 2 years ago

@adrien2p I don't think the issue is fixed yet, as we are still facing this issue. can you please let it open so that their might be a solution shared by team. 😊

Thanks

adrien2p commented 2 years ago

Sorry, i closed it because it is more related to typeorm and did not see any activities 😅

Also, i ve put on hold the migration to typeorm 3 in the core as it was collided a lot with our feature rate 🥹

abdokouta commented 1 year ago

@adrien2p There is an issue when 2 modules override the same entity, i believe the issue in databaseLoader function any recommendations?