-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
Description
Issue Description
Expected Behavior
After reading about the partial-update feature, where a literal null is required to save NULL to the database, and undefined indicates the field should be skipped entirely... I expected similar behavior when reading the NULL back from the database: a Javascript null in all cases.
Specifically, I expected to be able to define my nullable field as Category | null and guarantee that the value will only ever be a Category instance or a null.
Actual Behavior
Lazy relations that are not in the 'relations' find option return a Promise that resolves to a Javascript undefined. The other scenarios return a null.
Since Typescript is unaware of this unexpected behavior, there are no compile-time errors. But this inconsistency causes problems later when the actual value of the variable does not match the type specification, so we have to use Category | null | undefined all over the place to account for both possibilities.
While I don't have much of a preference of null vs. undefined, the method of retrieval (lazy-loaded vs lazy-loaded but included in the 'relations' find option vs. eager-loaded) should not change the outcome. That inconsistency is what I'd consider a bug.
Steps to Reproduce
This is a somewhat-contrived example, just to illustrate:
@Entity()
export class Category {
@PrimaryGeneratedColumn()
id!: number;
}
@Entity()
export class Post {
@PrimaryGeneratedColumn()
id!: number;
@ManyToOne(type => Category, { eager: true })
eagerCategory!: Category | null;
@ManyToOne(type => Category)
lazyCategory!: Promise<Category | null>;
}function verifyCategoryIsNull(category: Category | null) {
if (category === null) {
console.log('Oops! Post has no category');
} else {
console.log(`Awesome. Post has a category: ${category}`);
}
}
let post: Post;
post = new Post();
await connection.manager.save(post);
// Completely lazy
post = await connection.manager.findOneOrFail(Post);
verifyCategoryIsNull(await post.lazyCategory);
// Lazy by definition, but marked for eager-loading at the query level
post = await connection.manager.findOneOrFail(Post, { relations: ['lazyCategory'] });
verifyCategoryIsNull(await post.lazyCategory);
// Always eager, so no need for a Promise at all
post = await connection.manager.findOneOrFail(Post);
verifyCategoryIsNull(post.eagerCategory);Here is the output:
Awesome. Post has a category: undefined
Oops! Post has no category
Oops! Post has no category
Yes, I am aware that I could just replace === with ==, since that happens to match both null and undefined. Or I could do if (!category) and accept that it also matches other stuff. But a prominent purpose for using Typescript in the first place is to implement strong typing across the codebase. Ending up with a value that doesn't match the type specification is a problem.
My Environment
| Dependency | Version |
|---|---|
| Operating System | Ubuntu |
| Node.js version | v15.3.0 |
| Typescript version | v3.6.5 |
| TypeORM version | v0.2.29 |
Additional Context
I am using strict mode in Typescript.
Relevant Database Driver(s)
I demonstrated the failure on the following drivers, though it likely affects all of them; I don't think this is a driver-specific issue.
-
aurora-data-api -
aurora-data-api-pg -
better-sqlite3 -
cockroachdb -
cordova -
expo -
mongodb -
mysql -
nativescript -
oracle -
postgres -
react-native -
sap -
sqlite -
sqlite-abstract -
sqljs -
sqlserver
Are you willing to resolve this issue by submitting a Pull Request?
- Yes, I have the time, and I know how to start.
- Yes, I have the time, but I don't know how to start. I would need guidance.
- No, I don't have the time, although I believe I could do it if I had the time...
- No, I don't have the time and I wouldn't even know how to start.
I've already forked the repo, fixed the issue, and prepared a PR and unit suite to hilight the issue. Working on some final touches, and I'll post when that is complete.