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

0.4.0 release TODOs #3251

Open
4 of 18 tasks
pleerock opened this issue Dec 14, 2018 · 76 comments
Open
4 of 18 tasks

0.4.0 release TODOs #3251

pleerock opened this issue Dec 14, 2018 · 76 comments

Comments

@pleerock
Copy link
Member

pleerock commented Dec 14, 2018

I'm going to keep writing here features and changes I'm planning to introduce in 0.4.0:

  • (breaking) instead of named connections as they are right now, make it possible to write connections, managers and repositories into exported variables, e.g. const connection1 = createConnection(); const connection2 = createConnection();, const manager1 = connection1.manager; const manager2 = connection2.manager;, etc.
  • (breaking) we need to change some names in the project to make things more clear. Connection should be renamed to Storage and QueryBuilder should be renamed to Connection.
  • (breaking) remove transaction decorator ?
  • (breaking) remove custom repository functionality?
  • (breaking) deprecate eager and lazy loading?
  • (breaking) re-work current configuration loading and ormconfig file concept
  • re-work entity schemas and make them "primary way of defining a database schema model"
  • re-work QueryBuilder concept
  • make subset of changes regarding to how joins are used right now to implement entity streaming
  • provide a tutorial on how it can be easy to integrate typeorm and graphql
  • check if typeorm can be used with knex and provide a tutorial if possible
  • transpile it to es6 and support only node 10 to improve performance and make it easier to debug typeorm-based apps
  • add flag in find options to log query per-find-call
  • remove some magic methods from QueryBuilder (like joinAndMap)
  • add saveAndRemove method
  • (breaking) remove update and insert methods from repository because they confuse users
  • (breaking) somehow deprecate dynamic loading of entities (and others) from directory (or move this functionality into some package or module)
  • think if we can extract some cpu-intensive operations into threads (also resolve sqlite-related issues)

The list will be appended as we go.
My overall strategy for future typeorm versions is to redesign it to make it possible to use more advanced JavaScript features, make typeorm more functional (rather then classical oop like it is mostly right now), and use latest JavaScript and TypeScript features.

Feel free to discuss / start work on any of them!

@pleerock pleerock pinned this issue Dec 14, 2018
@JustinElst
Copy link

JustinElst commented Dec 14, 2018

(breaking) remove custom repository functionality?
(breaking) deprecate eager and lazy loading?

What would be the new way of doing this kind of stuff?

@havenchyk
Copy link
Contributor

@pleerock it would be nice to point to the proper github issues with some description, so community will be able to help you

@pleerock
Copy link
Member Author

@havenchyk its like building a puzzle, find everything and connect each peaces with each other and explain why I came up to this conclusion :) But I'll do it as we go.

@MrGekko there is a bit discussion in #2966 please check that

@samjmck
Copy link

samjmck commented Dec 14, 2018

(breaking) deprecate eager and lazy loading?

Do both of these things have to go? I read the discussion and it seemed like you were only talking about lazy loading, unless they are more or less the same thing code wise.

Also, this will make BaseEntity.reload() less useful. Could be fixed by looking into #3255

@pleerock
Copy link
Member Author

@samjmckenzie yeap, so this kind of "side effects" eager loading brings us - somebody don't want to load it in "reload", somebody don't like that eager isn't flexible and asks for "condition" feature request.

@samjmck
Copy link

samjmck commented Dec 14, 2018

@pleerock not sure I completely understand what you're saying

@Kononnable
Copy link
Contributor

What about #2479? It is marked as 0.3.0 but there is no clear solution yet.

@Kononnable Kononnable added this to the 0.4.0 milestone Dec 15, 2018
@rustamwin
Copy link
Contributor

How about add support mongodb 4 compatibility

@kaimiyang
Copy link

nover use custom repository functionality!!!

@pleerock
Copy link
Member Author

What about #2479? It is marked as 0.3.0 but there is no clear solution yet.

this is a complex one, we can move it into 0.4.0 but it can also be in 0.5.0

How about add support mongodb 4 compatibility

mongodb is community support feature :) feel free to implement task on it :)

@rustamwin
Copy link
Contributor

Idea: merge *Update and *Insert events into *Save event

@pleerock
Copy link
Member Author

save already does it.

@rustamwin
Copy link
Contributor

@pleerock #1527 (comment)

@kamilmysliwiec
Copy link

(breaking) remove custom repository functionality?
(breaking) deprecate eager and lazy loading?

We are using these both very heavily in a few applications. Could you please update this post with issue references so I could better understand why they have to be removed at some point? (eager + custom repository). I know that OSS is hard, although, it would help a lot of people I believe (including me 😅)

yeap, so this kind of "side effects" eager loading brings us - somebody don't want to load it in "reload", somebody don't like that eager isn't flexible and asks for "condition" feature request

Actually, I was also thinking a couple of times how to conditionaly "disable" eager loading. However, I don't think that removing this option entirely makes real sense. Even though it gives some "community side effects" and redundant questions, it reduces a boilerplate a lot 🙂

Thanks @pleerock for all of your work, love typeorm.

@pleerock
Copy link
Member Author

@kamilmysliwiec thanks, your feedback is very important here, since your words are words of thousand nestjs users I believe.

First, I would like to say that its on 0.4.0 roadmap (0.3.0 still isn't there) and for now I just use this issue to track my thoughts. So changes are far away from becoming real yet. Second, for all breaking functionality I would like to provide alternative solution (it can be more verbose or have a bit different syntax, but basically it will do the same). Also I'll try to provide a detailed motivation of why I decided to remove them.

Regarding to eager relation, I can tell you right now - its a bad pattern, I always didn't like it (long time before typeorm existence), and time only approved it. Some boilerplate just worth being in there because they can produce some good benefits. I imagine eager loading usage only in a very small few entity projects.

@kamilmysliwiec
Copy link

Cool, thanks for quick response! Looking forward to the new great things :)

Regarding to eager relation, I can tell you right now - its a bad pattern, I always didn't like it (long time before typeorm existence), and time only approved it. Some boilerplate just worth being in there because they can produce some good benefits. I imagine eager loading usage only in a very small few entity projects.

I don't like it either, however, it makes building small CRUD apps (especially great for workshops) much faster (prototyping purposes etc). I hope that there will be some sort of easy replacement for this functionality :)

@rustamwin
Copy link
Contributor

save already does it.

So what's the difference between *Update and *Insert events?

@ematipico
Copy link

@rustamwin From the documentation:
update and insert

    /**
     * Inserts a given entity into the database.
     * Unlike save method executes a primitive operation without cascades, relations and other operations included.
     * Executes fast and efficient INSERT query.
     * Does not check if entity exist in the database, so query will fail if duplicate entity is being inserted.
     */
   
    /**
     * Updates entity partially. Entity can be found by a given conditions.
     * Unlike save method executes a primitive operation without cascades, relations and other operations included.
     * Executes fast and efficient UPDATE query.
     * Does not check if entity exist in the database.
     */

Instead the save

    /**
     * Saves a given entity in the database.
     * If entity does not exist in the database then inserts, otherwise updates.
     */

@rustamwin
Copy link
Contributor

@ematipico see #1527 (comment)

@wzquyin
Copy link

wzquyin commented Dec 20, 2018

@pleerock
i pull a request about mongo4 transation support .
#3295
my code is very poor ....maybe the request is useful

@liangchunn
Copy link

Kinda off topic but:

Does it make sense to bump a minor version for breaking changes? Wouldn't a major version bump be better?

@pleerock
Copy link
Member Author

@liangchunn in 0.x.x second number means breaking changes, so you can treat it as major.

@liangchunn
Copy link

liangchunn commented Dec 29, 2018

@pleerock I see. I was expecting semversioning since installing packages with npm install or yarn add will add a carat prefix (like ^0.3.9) and will match any 0.x.x releases when upgrading automatically (like with the usage of Greenkeeper).

EDIT: of course people can do ~0.3.9 as well, but that’s just not the defaults

@ajkamel
Copy link

ajkamel commented Dec 29, 2018

https://semver.org/

@ryuuji3
Copy link

ryuuji3 commented Jan 4, 2019

The choice to use eager loading and lazy loading ultimately comes down to the developer. While it may be a bad pattern in your opinion, I do not agree that you should remove it. After all, if the functionality already exists, why remove it? If it's a matter of maintenance, then let's figure out a solution that even someone can put forth a PR.

As for custom repositories, I would also like to know the reasoning behind this!

@pleerock
Copy link
Member Author

pleerock commented Jan 4, 2019

@ryuuji3 it might seem less complicated than it is.

The choice to use eager loading and lazy loading ultimately comes down to the developer.

Its our responsibility to impose patterns that can lead to bad architecture or issues in the future. You can say its a "bad pattern in my opinion", but I would call it its a bad pattern in most use cases. Some patterns are very dangerous as they might seem good from a first sight, but later they show all their issues - its my responsibility to prevent such cases. At the end its all comes to down to the developer - if they want something specific - they can implement it on their own. For example eager loading can be implemented just by including all needed relations in the find options. Need to reuse? - Create constant with those relations. Need to reuse even more - create a separate find method with loading of those relations. Everything is much more flexible and explicit. Regarding to lazy loading feature - it was introduced as an experimental feature and now we totally understand that it can't fit any good architecture and only bring inconvenience and lot of issues when using other ORM features. Its also worth mention that sometimes you have a features A and B. But then you want to make really good features C and D, but they don't play well with feature B. Then you have to re-evaluate value of each feature and realize that implementing features C and D and maybe even E worth removing feature B. Sorry if its all too abstract, but if I'll write everything in details it will take me forever.

As for custom repositories, I would also like to know the reasoning behind this!

because it is redundant for typeorm functionality. In the future TypeORM versions I would go away to classic oop-style patterns that used in TypeORM and that do not fit well into JavaScript semantics. I'll provide deeper explanations later.

0.4.0 is long way feature, but here in this post I just want to warn users to prevent using features that will most likely be deprecated or removed.

@Kononnable
Copy link
Contributor

As for lazy loading we cannot say that we're removing that functionality. Entities still could be loaded lazily, but with different syntax. It turned out that introducing promises in entity definitions wasn't a good idea and it brings a lot of problems(it's just one part of the lazy loading problems).

@osman-mohamad
Copy link

osman-mohamad commented Dec 7, 2019

hey @pleerock , I want to start to use typeorm as the main db layer pakage . and it is very good package . that I can compare to eloquent laravel.
is this issue and it details still relevant ? or did you change your mind about this breaking changes ?

thank you.

@bashleigh
Copy link

bashleigh commented Dec 19, 2019

Is there any chance I can add polymorphic relationship handling into the Repository class? I have a work in progress that works quite well. It's not great but it works! (Not completed but you'll get the idea) https://medium.com/@ashleighsimonelli/handling-polymorphic-relationships-with-typeorm-96ad63cb8d0b


Edit

I've now released this as typeorm-polymorphic https://github.com/bashleigh/typeorm-polymorphic

@constb
Copy link
Contributor

constb commented Jan 16, 2020

@pleerock

(breaking) remove update and insert methods from repository because they confuse users

Hi. I'd like to object to this one. I'm using .insert()/.update() heavily as a lightweight alternative to .save(). Usually, even without cascade relations, .save() produces 3 queries: 1) check, 2) insert, 3) load generated columns.

There are cases when I return an entity from the request and this behaviour is desired. But another common use case is migration scripts where I process millions of records and 1) I know already that entity is missing in the table (no need to check) and 2) I don't care about generated columns (don't need to load).

In fact I was looking for a way to disable loading generated columns and there seems to be none. Only if I pre-generate it in code that calls insert, I end up with single INSERT statement.

Same goes to distinctAlias-selects when a query contains joins and limit – sometimes I want to tell TypeORM: "Hey, I swear those joins never generate multiple rows, just apply limit to the query itself!", but I can't…

I'm also missing INSERT IGNOREs and INSERT … ON DUPLICATE KEY UPDATE …. I know they are very mysql-specific and probably no ORM supports them anyway, but those would be great to have…

@cesc1802
Copy link

how about some issue as columns name too long ?

@jbjhjm
Copy link
Contributor

jbjhjm commented Apr 29, 2020

Is there any chance I can add polymorphic relationship handling into the Repository class? I have a work in progress that works quite well. It's not great but it works! (Not completed but you'll get the idea) https://medium.com/@ashleighsimonelli/handling-polymorphic-relationships-with-typeorm-96ad63cb8d0b

Great work, I used pretty much the same solution to implement a custom orderable many2many relationship. However there I have two concerns about that concept:

  1. Custom Repositories might be removed in the future
  2. each such feature require a specialized repository so they can't be used together. For example TypeOrm already comes with another beta repo specializing in tree relations.

It works for now... but I wish typeOrm had something like a plugin system to extend data management flexibly. The repository class might be the best place to add such functionality.

@eporomaa
Copy link

eporomaa commented Jun 5, 2020

@pleerock

(breaking) remove update and insert methods from repository because they confuse users

We use insert to ensure that we do not perform any updates on data. With only save it would be easier for a developer to make a mistake and update a row when it should not be. Please don't remove this optionality.

@Delfio
Copy link

Delfio commented Aug 19, 2020

(breaking) remove custom repository functionality?

I have a question. Is removing the custom repository classes that modify the default repository directly? say for example this class
@EntityRepository(Y) class x extends Repository<Y>{...}

Because I have some projects that have their own implementation of repositories, because I have entities that do not directly match the database tables (due to the business rule). It does not follow this pattern shown above. And I was wondering if this pattern of my project will be discontinued. Follow the code

@kikar
Copy link
Contributor

kikar commented Nov 11, 2020

transpile it to es6 and support only node 10 to improve performance and make it easier to debug typeorm-based apps

@pleerock according to node.green Node.js 10 fully supports up to ES2017 included. Why only stopping at ES2015 (ES6)?

@ginachoi
Copy link

@kamilmysliwiec thanks, your feedback is very important here, since your words are words of thousand nestjs users I believe.

First, I would like to say that its on 0.4.0 roadmap (0.3.0 still isn't there) and for now I just use this issue to track my thoughts. So changes are far away from becoming real yet. Second, for all breaking functionality I would like to provide alternative solution (it can be more verbose or have a bit different syntax, but basically it will do the same). Also I'll try to provide a detailed motivation of why I decided to remove them.

Regarding to eager relation, I can tell you right now - its a bad pattern, I always didn't like it (long time before typeorm existence), and time only approved it. Some boilerplate just worth being in there because they can produce some good benefits. I imagine eager loading usage only in a very small few entity projects.

We are in process of evaluating TypeORM to use in our applications. I am coming from Hibernate/JPA/Spring Data background and custom repository is very useful in our applications. We will create custom repositories and store them in our container inject them in the service layer. Is there any reason to remove custom repository? TypeORM Repository class won't provide all APIs needed for applications.

@codeninja
Copy link

codeninja commented Mar 27, 2022

Is there any chance I can add polymorphic relationship handling into the Repository class? I have a work in progress that works quite well. It's not great but it works! (Not completed but you'll get the idea) https://medium.com/@ashleighsimonelli/handling-polymorphic-relationships-with-typeorm-96ad63cb8d0b

Edit

I've now released this as typeorm-polymorphic https://github.com/bashleigh/typeorm-polymorphic

@bashleigh With the latest release of Typeorm@ 0.3.4 This plugin is now broken. I am working locally on a fix and will update this comment if and when my PR is approved.

@pleerock is polymorphisim on the table for an approved feature to roll into the main typeorm Repository? If so I'd rather put my efforts towards adding the feature in typeorm rather than patching BashLeigh's work.

Feel free to dm.

@pleerock
Copy link
Member Author

@pleerock is polymorphisim on the table for an approved feature to roll into the main typeorm Repository? If so I'd rather put my efforts towards adding the feature in typeorm rather than patching BashLeigh's work.

@odeninja we don't have such plans in a feasible future

@codeninja
Copy link

@codeninja we don't have such plans in a feasible future

Would this be a feature you'd be interested in if I could hand you a solution?

@pleerock
Copy link
Member Author

pleerock commented Apr 5, 2022

Since our main strategy right now is to keep core codebase simple and maintainable, I suggest to develop third-party plugins or integrate it into exist ones.

@danielkochdakitec
Copy link

danielkochdakitec commented Sep 9, 2022

@pleerock

(breaking) remove update and insert methods from repository because they confuse users

We use insert to ensure that we do not perform any updates on data. With only save it would be easier for a developer to make a mistake and update a row when it should not be. Please don't remove this optionality.

We also use insert and update since it is a lightweight alternative to save. We use update() to specify only the fields we want to update since save() requires the whole object which does not work in our case because of concurrency. Please don't remove update() and insert().

@vicary
Copy link

vicary commented Jan 9, 2023

Support Tree-Shaking for decorators

  1. Importing decorators will force decorator itself into the final bundle, without any actual usage.
  2. Re-exporting entity classes in a file (e.g. index.ts) will force all of them the final bundle.

Minimum repo: https://stackblitz.com/edit/node-apuuys?file=utils%2FTestModel.ts

Support Tree-shaking for drivers

I think we should revisit the possibility of #3469.

Including every single driver results in greatly increased cold-start times for AWS Lambda (from 150ms to 800ms), this is especially cost ineffective when Lambda usually uses HTTP proxies when connecting to databases.

In theory, AWS Lambda, when working with HTTP proxy drivers such as AuroraDataApi can depend only on fetch() and shakes away all other drivers which depends on TCP/Unix sockets.

@daitay4
Copy link

daitay4 commented Jan 27, 2023

Removing insert and update (and presumably upsert?) does not sit well with me tbh.

Any good reason for that other than "it confuses users"?

@alenap93
Copy link
Contributor

what are the most interesting features that we can implements for 0.4.0?

@Nosfistis
Copy link

Stricter types

I have recently worked with Supabase, and its TS types are amazingly good (works great with strict typescript).
#10082 is a good start.

saveNew

This could be an extra argument in save() or insert() as well. Sometimes I would like to save an entity only if it not already there (expecting a unique index constraint to fail). save() will just overwrite the entity. insert() will fail, but in case of success there is no way to also return the saved entity, and InsertResult is returned. Unless there is something else I am missing.

Watch

Now this can also be achieved with Subscribers, but I think a Repository should offer easier ways to listen to changes (node callback?). I have already implemented such a pattern with a Subscriber and an RxJS Subject.

Migration lock

In case of running a NodeJS process in cluster mode, TypeORM needs to be configured to run migrations only in one instance. Currently I use NODE_APP_INSTANCE or some other cluster identifier to identify if this is the first instance, and set migrations to run only there. Perhaps there should be a lock column in the migrations table while a table runs migrations?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests