I'm going to keep writing here features and changes I'm planning to introduce in 0.4.0:
const connection1 = createConnection(); const connection2 = createConnection();, const manager1 = connection1.manager; const manager2 = connection2.manager;, etc.Connection should be renamed to Storage and QueryBuilder should be renamed to Connection.saveAndRemove method update and insert methods from repository because they confuse usersThe 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!
(breaking) remove custom repository functionality?
(breaking) deprecate eager and lazy loading?
What would be the new way of doing this kind of stuff?
@pleerock it would be nice to point to the proper github issues with some description, so community will be able to help you
@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
(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
@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.
@pleerock not sure I completely understand what you're saying
What about #2479? It is marked as 0.3.0 but there is no clear solution yet.
How about add support mongodb 4 compatibility
nover use custom repository functionality!!!
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 :)
Idea: merge *Update and *Insertevents into *Save event
save already does it.
@pleerock https://github.com/typeorm/typeorm/issues/1527#issuecomment-447701324
(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.
@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.
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 :)
save already does it.
So what's the difference between *Update and *Insert events?
@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.
*/
@ematipico see https://github.com/typeorm/typeorm/issues/1527#issuecomment-447701324
@pleerock
i pull a request about mongo4 transation support .
https://github.com/typeorm/typeorm/pull/3295
my code is very poor ....maybe the request is useful
Kinda off topic but:
Does it make sense to bump a minor version for breaking changes? Wouldn't a major version bump be better?
@liangchunn in 0.x.x second number means breaking changes, so you can treat it as major.
@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
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!
@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.
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).
So what's the best alternative to lazy loading?
@ematipico new api will be developed instead of current lazy loading mechanizm
What about 0.3.0 version? New 0.4.0 will begin from master or next branch? In next branch rewriten the find options and it is cool feature.
@quaternion work on 0.3.0 is in progress. The main feature of 0.3.0 are find options. Once 0.3.0 will be released I'll start working on 0.4.0. This issue is just to track tasks for 0.4.0.
@pleerock
Thank you for your reply! I only said it like that because it seemed as though it was more your personal opinion than anything.
I have already removed all eager loading from the project and experimented with it. I arrived at the same conclusion to store a list of relations that would always need to be loaded.
However this presented new difficulties.
I'll explain a bit better.
Starting with an abstract service:
export abstract class AbstractService<T extends AbstractEntity<T>, I = any> {
constructor(protected repository: Repository<T>) { }
abstract findOne(user: User, id: UUID, options?: FindOneOptions<T>): Promise<T>
abstract findAll(user: User, options?: FindManyOptions<T>): Promise<T>
// CRUD Methods
async create(user: User, payload: I): Promise<T> { ... }
// Example implementation that uses abstract methods
async delete(user: User, id: UUID): Promise<T> {
const found = await this.findOne(user, id);
return this.repository.remove(found);
}
}
And a service that uses it
export class UserService extends AbstractService<User> {
async findOne(user: User, id: UUID, options?: FindOneOptions<User>) {
if (user.role === UserRole.ADMIN) {
return this.repository.findOne(id, options);
}
return this.repository.findOne(id, {
...options,
where: { role: UserRole:BASIC }
});
}
}
In this contrived example, there are only two user roles. So I want to make sure that basic users can only see other basic users, not admins that they shouldn't know about.
When I remove eager loading, I then made a static getter that contained all the FindOneOptions<User> with relations that I always want to load.
export class UserService extends AbstractService<User> {
private static options(): FindOneOptions<User> {
relations: ["posts"]
}
async findOne(user: User, id: UUID, options?: FindOneOptions<User>) {
const findOptions = { ...UserService.options, ...options);
if (user.role === UserRole.ADMIN) {
return this.repository.findOne(id, findOptions);
} else {
return this.repository.findOne(id, {
...findOptions,
where: { role: UserRole:BASIC }
});
}
}
}
This isn't perfect but it worked really good! Edit: Until I needed to use Query Builder!
The incompatibility of QueryBuilder and Find*Options<T> presented a challenge. I ended up having to translate FindOneOptions.relations;
const alias = `User`;
const qb = this.createQueryBuilder(alias);
if (options.relations) {
options.relations.forEach(path => {
const paths = path.split(".");
const pathAlias = paths[paths.length - 1];
qb.leftJoinAndSelect(path, pathAlias);
});
}
That way I could at least use the one Find*Options
Is there something I could have done better?
The solution that makes the most sense was when I removed a relationship from my static getter and went with something simple;
{ eager: true }
That was it. No headaches. Simple. And something that makes sense. And it's also compatible with Find*Options
loadEagerRelations: false
So that you can load the relations you want specifically if not the default ones that you need 99% of the time.
That's been my experience in a world without eager relations. I think that there are times they make sense. Which is why I said that it should be up to the discretion of the developer.
Is there something I could have done differently?
Until I needed to filter a M:N relationship. Unfortunately, it appears that using find* operators are not fully supported! And FindOptions is only supported by the find operators. The incompatibility of QueryBuilder and Find*Options
presented a challenge.
sorry but I didn't understand what this means. Looks like a question that runs out of scope of this issue. Let's keep this issue cleaner and more focused :)
@pleerock
The issue was not the focus of the statement. The focus was that currently, for whatever reason, the interface FindOneOptions<T> is not compatible with QueryBuilder.
I only mentioned it as way of explanation.
I edited it out, though. Can we pretend it didn't exist and continue the discussion? :D
@ryuuji3 my point is that you might misunderstood something, or I did and it shall be a separate discussion since we are trying to find a solution with or without new api to your problem. Its better to discuss such things in a separate issue, I don't want to do it here. I still don't understand what you mean by interface FindOneOptions<T> is not compatible with QueryBuilder - yeah they are different, because they are different classes, but you still can do everything you can do with find options in the query builder, by calling different methods.
We are using the lazy feature for all of our relations with GraphQL, via https://github.com/19majkel94/type-graphql.
The point being that each GraphQL query will load the relations only if needed, calling await entity.relation each time. Since we do not know in advance what relation do we need to preload.
It may not be the best implementation but it works fine in this context with GraphQL. If you change the pattern could you please take this kind of flexibility into account ?
Maybe the lazy feature could be implemented as a plugin?
@RuslanPopchef sure, we will provide mechanizms that can be used by type-graphql as well
To support tree-shaking (avoid needless drivers and reduce webpack bundle size) we dispose of DriverFactory. So we could explicitly import only needful drivers:
import {createConnection} from "typeorm/browser";
import {NativescriptDriver} from "typeorm/browser/driver/nativescript";
const connection = await createConnection({
database: 'test.db',
driver: NativescriptDriver,
entities: [
Todo //... whatver entities you have
],
logging: true
});
All other necessary imports for driver should be done by corresponding driver (for NativescriptDriver it should be nativescript-sqlite npm-package... user should only install this npm-package and should not import it explicitly).
But as I can see, for example, PostgresDriver is imported in InsertQueryBuilder. There are should not be such things (Driver interface is done for that). All specific for driver things should be implemented in postgres directory and should not be imported elsewhere except postgres dir.
@webleaf good points. But package separation is planned for post 0.4.0 versions.
I disagree wholeheartedly with your decision regarding removing eager-loading. I feel that your decision to move away from traditional OOP like functionality is an extremely bad move.
My team currently uses TypeORM in a moderately sized production commercial application ( ~10k users ). We came from a wide variety of technical backgrounds, some of us from a more traditional OO background in Java/.net, some from a more modern Node background. We compromised on Typescript/TypeORM solely on the basis of its support for OOP like principles that we were already familiar with.
Support is still lacking in some key areas, but overall we've been very impressed with the ORM and grateful for the work of the developers. I find that the decisions to move away from supporting OOP like paradigms extremely disheartening, and withdrawing support from more traditional features causes me some concern regarding the long-term viability of using TypeORM in this and other projects.
I understand that the platform imposes some limitations on what can be accomplished in this area, but what impressed me initially about this project was that its support of the repository design pattern out of the box lent itself to supporting traditional OOP design patterns centred around _persistence ignorance_.
Ideally, the ORM should play an extremely minimal role in the business logic of the application. I should not be _relying_ on it to manage our business logic every step of the way. This is absolutely an anti-pattern.
Ideal OOP design in this area should consist of using repositories only for serialising and deserialising _aggregate entities_. I strongly feel that explicitly managing loading and individual persisting relations is a step in the entirely wrong direction.
I feel that your decision to move away from traditional OOP like functionality is an extremely bad move.
can you justify why it is a bad move?
My team currently uses TypeORM in a moderately sized production commercial application ( ~10k users ). We came from a wide variety of technical backgrounds, some of us from a more traditional OO background in Java/.net, some from a more modern Node background. We compromised on Typescript/TypeORM solely on the basis of its support for OOP like principles that we were already familiar with.
I guess keyword here is you are familiar with. When moving from one platform to another, from one tool to another you always seek for something that is familiar. But you still need to learn something new. You found such tool already. Now you'll only need to follow new changes and adopt them. Its incremental to you (since you already in) and doesn't harm your team. It's a progress.
Support is still lacking in some key areas
you probably saw a post about donations, let me know if you already a donator (I'll prioritize support for your team).
I find that the decisions to move away from supporting OOP like paradigms extremely disheartening, and withdrawing support from more traditional features causes me some concern regarding the long-term viability of using TypeORM in this and other projects.
its your personal opinion and you can be wrong or right and based and your wrong or right decisions (which is relative of course) can affect this and other projects. If you think other than OOP paradigms are worse than I think you are absolutely wrong. If you don't think so then what it is disheartening for you?
I understand that the platform imposes some limitations on what can be accomplished in this area
platform imposes not only limitations - it also exposes new opportunities that can't be accomplished on other platforms. That's why it's not always have to be same.
traditional OOP design patterns
time to move forward. Traditional doesn't mean to be best. We can do things better with new technologies in mind.
eager-load
personally I think eager loading is an anti-pattern with all "classic OOP principles" in place. But I've got your and others feedbacks and I'm not sure if I'll remove it now. At least I'll provide some alternative. We can discuss it in details once I do it.
I came from the same background as you, that's why I created this tool, and that's why it looks so familiar to you. But time, learning new technologies and principles showed me that it's not the best way to do things. Everybody thinks/judges on his own experience.
can you justify why it is a bad move?
I already mentioned that I feel that an ORM should play an entirely minimal role in the business logic of the application. Its role should be limited to serialisation and deserialisation of _aggregate objects_. Parent->Child relationships are essentially the core of the conceptual model that is relational database schema design. This model maps very well to OO paradigms, which is why RDBMS->ORM->OO has become such a standard in the industry for enterprise application architecture.
DDD also became so widely adopted because this approach to modelling business logic is so intuitive.
Customer has 1...n Orders. An Order has 1...n Items... etc.
Typescript is fundamentally object oriented, and the very definition of ORM implies object orientated programming too, I'm not sure why I need to defend OO here.
I guess keyword here is you are familiar with. When moving from one platform to another, from one tool to another you always seek for something that is familiar. But you still need to learn something new. You found such tool already. Now you'll only need to follow new changes and adopt them. Its incremental to you (since you already in) and doesn't harm your team. It's a progress.
I'm already very familiar with the Node ecosystem, for better or worse. I'm arguing my position on the basis that, being familiar with both OO and non-OO approaches for modelling relational structures in code, I find the OO approach much more intuitive. It's not that we just can't figure out how to accomplish things in this manner, it's that we disagree with the direction you're proposing.
its your personal opinion and you can be wrong or right and based and your wrong or right decisions (which is relative of course) can affect this and other projects. If you think other than OOP paradigms are worse than I think you are absolutely wrong. If you don't think so then what it is disheartening for you?
It's about choosing the right tool for the job. If I got to choose what kind of programming I did professionally I probably wouldn't be programming in OO to be honest. It's not that I'm a zealot who only understands one mode of thinking. I've had a reasonable amount of experience in this area and I just feel that I'm championing the approach that has worked best for many projects.
I came from the same background as you, that's why I created this tool, and that's why it looks so familiar to you. But time, learning new technologies and principles showed me that it's not the best way to do things. Everybody thinks/judges on his own experience.
I appreciate this. I also appreciate that I don't know everything and I try to keep an open mind. Given that I've tried to make my case for why I disagree, can you justify _why_ you think eager-loading is an anti-pattern? And if so, what approach should we be taking if not the approach of loading aggregate root entities?
I already mentioned that I feel that an ORM should play an entirely minimal role in the business logic of the application.
yes you are right, it should, but its not related if orm is OO or something else
DDD also became so widely adopted because this approach to modelling business logic is so intuitive.
you are right, but DDD isn't related to OO, so everything will stay same in this aspect.
This model maps very well to OO paradigms, which is why RDBMS->ORM->OO has become such a standard in the industry for enterprise application architecture.
I didn't say that I'll go to far away from OO. Here you are talking about object relations and they will stay. OO can mean lot of things, and for many people it can mean more than it actually is. I'm sure if we continue this conversation in details we can came up to the big list of what OO is or not and can agree and disagree on some them. Again, object relations will stay.
Typescript is fundamentally object oriented, and the very definition of ORM implies object orientated programming too, I'm not sure why I need to defend OO here.
its not right. TypeScript is fundamentally is just about adding types to JavaScript and it doesn't imply you to OOP. TypeScript three years ago and before was OOP focused and it's obviously for everybody including TypeScript team was a wrong direction. If TypeScript do it again nowdays they won't include many features they did, but unfortunately it's too late already and they can simply get and remove some features. So, TypeScript is fundamentally same oriented as JavaScript and we all know JavaScript isn't only OO.
I'm already very familiar with the Node ecosystem, for better or worse. I'm arguing my position on the basis that, being familiar with both OO and non-OO approaches for modelling relational structures in code, I find the OO approach much more intuitive.
Simply because we always take with ❤️ everything we know and did before. It's natural to us. Once you get familiar with a new approaches and get understanding of everything new and better they bring you'll love them and start hate old approaches. Your disagree stays with the same motivation.
It's about choosing the right tool for the job. If I got to choose what kind of programming I did professionally I probably wouldn't be programming in OO to be honest. It's not that I'm a zealot who only understands one mode of thinking. I've had a reasonable amount of experience in this area and I just feel that I'm championing the approach that has worked best for many projects.
I understand your love for OOP. I completely understand it because I had this love too. And I still love many aspects of it. But I'm not fanatic and conservative about it anymore. FP and other concepts aren't get so much popularity nowadays. And btw, there are fanatics of FP in this repository who will say to remove many oo features, make everything immutable, etc. etc. I disagree with them too, I want to get best of both worlds and its hard to do.
can you justify why you think eager-loading is an anti-pattern?
I'm using eager loading for the last ten years in different projects and know its pros and cons. I added this feature to TypeORM only because it was easy to add and I wanted as more features as it is possible to add haha. Reason why I'm thinking it is bad is because its inflexible, its implicit and easily can be overused by team and in most times it brings issues. Inflexible design decisions and implicitness of code and behaviour (also complexity and overengineering) are REAL developer problems, not questions about paradigms. Why its inflexible? Because when you set eager: true on relation one, its always in there. Then you add eager: true to another relation inside this relation, then your entity and project structure becomes BIG and there are lot of eager relations and you are realize it brings you real performance problems. Then you try to remove some of them and you understand how problematic it is for you because you need to change every place they are used and make it same level of loading with having all a1->b1->..->zx relations in mind and obviously you are already in production. In practice its really hard to do. Most probably you'll start doing one-by-one and it will be a time-consuming process. And I promise during this period some people from your team will add eager relations back without knowledge why it was removed. And the biggest problem that it can be on any level of your relation tree. Having to specify a clear number of relations you need make your code more explicit, more flexible, you have more control and you improve application maintenance level. Then what is the point of eager relations? Obviously this pattern works only in small apps with few entities. But we all know any small project grows and exist code with exist eager relations won't be ever changed, and team will keep using eager relations since they used it from the beginning and one day you'll have those issues. I simply don't want orm to imply bad patterns.
@pleerock @ajxs
Let me interfere a bit.
Is I understand correctly, @ajxs idea is that ORM should not be visible in the business logic at all. No direct load() calls on relations etc, the _persistence ignorance_ in general. I think, this is very important.
Eager loading somehow manages to do it because you don't need to call ORM service methods and can focus on business logic only. But of course making all the relations eager is a potential disaster of loading entire database in an object graph in memory.
One solution is a manual setting of what relations you need on each find() call. But it definetly violates _persistence ignorance_ - just because the list of needed relation is out of the business domain model. No compiler can catch the places where you try to access a reation not mentioned in the _include: [rel1, rel2, ...]_ clause.
The better solution is automatic lazy loading. Say, you have a _book_ instance and want to get its _authors_. It would be great if when you load some _books: Book[]_ collection you can just access _.authors_ property like for (b in books) {for (a in b.authors) console.log(a.name)}.
Current lazy mechanizm requires to explicitly await promises that is also not very _persistence ignorance_ aware. And entire idea of declaring object relations differently for normal and lazy loaded relations looks bad.
Why not just implement some sort of lazy decorator? This way when you load a collection it doesn't load relations and only when you access a relation property for the first time the relation is loaded on demand.
So for example: books = booksRepo.find() doesn't load authors but as you execute books[0].authors it does for that particular book.
The simplest decorator can look like this:
export function lazyLoad(logging: boolean = false) {
return function (target: any, key: string) {
if (logging) console.log('lazyLoad setup: this:', this, '; target:', target, '; key:', key);
// property getter
var getter = function () {
if (logging) console.log('lazyLoad: Get: this:', this);
if (this['__' + key] === undefined) {
if (logging) console.log('lazyLoad: getting a value for the first time when it is undefined yet...')
// just place here the relation loading
this['__' + key] = ['Some heavy value', 'And another heavy value'];
Object.defineProperty(target, '__' + key, {
enumerable: false,
writable: true
});
};
return this['__' + key];
};
// property setter
var setter = function (newVal) {
if (logging) console.log('lazyLoad: Set: this:', this);
this['__' + key] = newVal;
};
// Create new property with getter and setter
Object.defineProperty(target, key, {
get: getter,
set: setter,
enumerable: true,
configurable: true
});
}
}
And now
class Book {
id: number;
name: string;
@lazyLoad() authors: Author[];
}
I see that all the DB access methods are async and return promises. So probably all the functions calling that methods must be declared as async as well. This is a problem. But may be it's possible to hide all that async staff in the decorator internally, so that for outside world the budiness object model is clear of all this technical things. I have not checked whether I can declare getter and setter for Object.defineProperty as async. May be it's possible...
May be it's possible...
Unfortunately not. Any attempt to hide _await_ inside getter fails.. No wonders.
@MaximS because you are working in async environment! wake up guys! 😆
class MyClass {
private async myAsyncFunction() { }
get asyncResult() {
return this.myAsyncFunction();
}
}
@ryuuji3 It makes no sense, because Promise will be returned (async return Promise). You just create alias without parentheses.
I wonder if we should implement typescript strict mode support to typeorm. Currently it is only possible to use this mode by adding ! to properties in entities(and lying to the compiler) or adding | undefined it types.
I think it could be a lot of work but this compiler option is getting more common.
@Kononnable may I ask question, why initializing all properties won't work? (e.g. make it id: number = 0; name: string = "", etc. )
We could create query builder from repository object(User) and select only few columns. Return type seen by typescript would be User, but some properties would be undefined(since we didn't fetch them from db).
If we initialize all properties we would have different data in local model and db(and I think doing save later would update db according to our default data).
We could create query builder from repository object(User) and select only few columns. Return type seen by typescript would be User, but some properties would be undefined(since we didn't fetch them from db).
but in this case it looks like our model was defined wrong, isn't it? If you suppose your user should't have some property (and you actually do in your case) - you should make that property optional.
@MaximS
No compiler can catch the places where you try to access a reation not mentioned in the _include: [rel1, rel2, ...]_ clause.
https://www.typescriptlang.org/play/index.html#src=type%20FindParams%3CT%20extends%20object%2C%20K%20extends%20keyof%20T%3E%20%3D%20%7B%0A%20%20%20%20params%3A%20K%5B%5D%3B%0A%7D%3B%0A%0Aclass%20Model%3CT%20extends%20object%3E%20%7B%0A%20%20%20%20select%3CK%20extends%20keyof%20T%3E(params%3A%20%7B%0A%20%20%20%20%20%20%20%20keys%3A%20K%5B%5D%2C%0A%20%20%20%20%7D)%3A%20Pick%3CT%2C%20K%3E%20%7B%0A%20%20%20%20%20%20%20%20return%201%20as%20any%3B%0A%20%20%20%20%7D%0A%7D%0A%0Aconst%20m%20%3D%20new%20Model%3C%7B%20a%3A%20string%2C%20b%3A%20number%2C%20c%3A%20string%20%7D%3E()%3B%0A%0Am.select(%7B%0A%20%20%20%20keys%3A%20%5B'a'%5D%0A%7D).b%3B
Why not?
Hmm, it may become harder for deep eager loading.
Maybe eager should be like?
eagerOption: {
depA: {
depB: ['a', 'b'] // as record or as string[]
}
}
@Kononnable may I ask question, why initializing all properties won't work? (e.g. make it
id: number = 0; name: string = "", etc.)
Might be a dumb question, but wouldn't that mess up the id generation? (eg what will actually happen to the id if creating a new User if I set id to 0?). Or Date columns such as created_at and updated_at?
can you justify why you think eager-loading is an anti-pattern?
I'm using eager loading for the last ten years in different projects and know its pros and cons. I added this feature to TypeORM only because it was easy to add and I wanted as more features as it is possible to add haha. Reason why I'm thinking it is bad is because its inflexible, its implicit and easily can be overused by team and in most times it brings issues. Inflexible design decisions and implicitness of code and behaviour (also complexity and overengineering) are REAL developer problems, not questions about paradigms. Why its inflexible? Because when you set
eager: trueon relation one, its always in there. Then you addeager: trueto another relation inside this relation, then your entity and project structure becomes BIG and there are lot of eager relations and you are realize it brings you real performance problems. Then you try to remove some of them and you understand how problematic it is for you because you need to change every place they are used and make it same level of loading with having all a1->b1->..->zx relations in mind and obviously you are already in production. In practice its _really_ hard to do. Most probably you'll start doing one-by-one and it will be a time-consuming process. And I promise during this period some people from your team will add eager relations back without knowledge why it was removed. And the biggest problem that it can be on any level of your relation tree. Having to specify a clear number of relations you need make your code more explicit, more flexible, you have more control and you improve application maintenance level. Then what is the point of eager relations? Obviously this pattern works only in small apps with few entities. But we all know any small project grows and exist code with exist eager relations won't be ever changed, and team will keep using eager relations since they used it from the beginning and one day you'll have those issues. I simply don't want orm to imply bad patterns.
Hey @pleerock, not sure if this is the appropriated issue to post about the topic, but I'm interested in this. I see huge benefits in both eager-loading and lazy-loading.
I for sure don't know all the details of your use case, but it seems weird to me that you would decide to remove the "eager" option from an entity when the business rules dictate that it should always be present (which I'm assuming is the reason we set the eager option). If you need to optimize, wouldn't making another entity, that does not eager-load dependencies make more sense?
Some months ago I was working on a relatively complex application that managed several kinds of entities. We had "Applications" which had "Questions" which had all sort of information associated with them. We even had a different kind of questions (Master Question, Carrier Question) that were associated with each one. We noticed that some parts of our application were becoming slow because we were loading a lot of unneeded data. The solution was creating a new entity, CarrierQuestionLite, that represents a Question but with as minimal information as needed. This is fair in DDD since the meaning of an entity is determined by the context, and for the context we wanted to optimize, we didn't need as much data as we did in the other context.
I completely understand that as an author you need to enforce best practices and discourage bad ones. Looking forward to seeing what the replacements will look like!
I think this really begs the question of "why introduce the repository pattern at all if you're going to attempt deprecating eager and lazy-loading of relations?".
The entire idea of the repository pattern is predicated upon the notion of persistence-ignorance. In an ideal world you could use the repository pattern with your entities, implement very complex business logic and be able to swap ORM with very little changes in the service layer ( You shouldn't to this though ). To explain:
The way the repository pattern is intended to work, as per my understanding is that the repository is responsible only for the persistence ( serialising/deserialising ) of aggregate entities. If you're confused about these terms consult the books written by Martin Fowler on the subject. Basically you deserialise an aggregate entity from the database, which loads the entity into memory in the form of an entity class instance. From there you use the instance methods to implement the business logic as per your requirements, then use the repository to serialise the entity again, which should cascade to nested relations. This is how the pattern was intended to work.
Service begins. Load entity once, modify entity as many times as needed, save entity once. Service ends.
My description above is not at all controversial, it's also not predicated upon using Java/C# either. This is an OO perspective, but the repository pattern is fundamentally object-oriented.
The repository is NOT supposed to simply be another query builder class.
As @clovis1122 above hinted, for many business cases there's no point having a parent entity without its child relations. For the project I use TypeORM in I'd end up writing hundreds of new lines just to satisfy base business requirements for simply loading otherwise. If TypeORM removed eager/lazy loading of relations altogether we'd be forced to completely ditch it in our project.
Just curious why you want to remove QueryBuilder.joinAndMap ?
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.
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
@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…
how about some issue as columns name too long ?
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:
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.
@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.
(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
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)?
Most helpful comment
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 😅)
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.