Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate Migration TypeORM #113

Open
juanzgc opened this issue Aug 14, 2022 · 15 comments
Open

Generate Migration TypeORM #113

juanzgc opened this issue Aug 14, 2022 · 15 comments
Labels
invalid This doesn't seem right need reproduction

Comments

@juanzgc
Copy link
Contributor

juanzgc commented Aug 14, 2022

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:

  • Product: [images, options, variants, profile, collection, type, tags, sales_channel]
  • User: []

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"
  }
}
  1. Add the following scripts in package.json:
    "typeorm": "node --require ts-node/register ./node_modules/typeorm/cli.js",
    "migration:generate": "yarn typeorm migration:generate",
  1. Install typeorm:
  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
  1. 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

@juanzgc
Copy link
Contributor Author

juanzgc commented Aug 14, 2022

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
Copy link
Owner

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 adrien2p added need reproduction invalid This doesn't seem right labels Aug 14, 2022
@adrien2p
Copy link
Owner

adrien2p commented Aug 16, 2022

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

medusajs/medusa#2054 (comment)

@juanzgc
Copy link
Contributor Author

juanzgc commented Aug 17, 2022

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.

@adrien2p adrien2p changed the title [BUG] Generate Migration TypeORM Generate Migration TypeORM Aug 17, 2022
@aboozaid
Copy link

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
Copy link
Owner

@juanzgc did you give it a shot 👆

@juanzgc
Copy link
Contributor Author

juanzgc commented Aug 20, 2022

I have not, will attempt this later tonight 👀

@ZiaSoltani2023
Copy link

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

@juanzgc
Copy link
Contributor Author

juanzgc commented Aug 22, 2022

@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
Copy link

aboozaid commented Aug 22, 2022

@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
Copy link
Contributor Author

juanzgc commented Aug 27, 2022

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
Copy link
Contributor Author

juanzgc commented Aug 27, 2022

It seems that in Typeorm 3 - this solution might work but we will have to wait until Medusa is migrated to use Typeorm v3: typeorm/typeorm#9122 (comment)

@ZiaSoltani2023
Copy link

@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
Copy link
Owner

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 🥹

@adrien2p adrien2p reopened this Oct 12, 2022
@abdokouta
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right need reproduction
Projects
None yet
Development

No branches or pull requests

5 participants