search

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
320 stars 40 forks source link

Feat/typeorm upgrade #180

Closed SGFGOV closed 1 year ago

SGFGOV commented 1 year ago

Hi,

I've made the changes and the unit tests pass. Please review the changes and let me know what you think.. will clean up the code once your ok with the changes.

adrien2p commented 1 year ago

Thanks for sharing 😊 There is multiple implementation issue that i can see. But I would like to discuss one in particular.

Why did you choose to go with extends Repository instead of extends MedusaOrderRepository? With the actual changes you loose the access to the parent class methods feom the child one and you have to assign the methods from the override class to the custom one during the loading. Whereas if you extended the MedusaOrderRepository you would have access to the parent class methods and you wouldn't have to assign the methods in the loader. Anything I am missing?

SGFGOV commented 1 year ago

Thanks for sharing 😊 There is multiple implementation issue that i can see. But I would like to discuss one in particular.

Why did you choose to go with extends Repository instead of extends MedusaOrderRepository? With the actual changes you loose the access to the parent class methods feom the child one and you have to assign the methods from the override class to the custom one during the loading. Whereas if you extended the MedusaOrderRepository you would have access to the parent class methods and you wouldn't have to assign the methods in the loader. Anything I am missing?

Thanks for your very prompt feedback. I struggled with this for a bit. If it was a class I could have pulled it rather easily and extend, infact no changes would be necessary.

However, in Medusa 1.8.x MedusaOrderRepository isn't a class.. Order repository is extended like this

export const OrderRepository = dataSource.getRepository(Order).extend({.... }) essentially an object, with prototype defined. the other route was to reflect, but felt that is adding complexity when not necessary.

Open to all suggestions and help :) I'm currently stuck with the loading sequence during integration

gitusertd commented 1 year ago

Seem to be a problem. package.json was changed to: "@medusajs/medusa": "npm:@sgftech/medusa"

@adrien2p: May I ask if there is any Work In Progress to upgrade TypeORM in Medusa Extender to v3 and match with Medusajs?

adrien2p commented 1 year ago

The extender will be deprecated since now we ve done everything to support it in medusa. You can read the doc about customisation 💪