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

feat(core): allow disabling identity map and change set tracking #1307

Merged
merged 1 commit into from
Jan 15, 2021

Conversation

B4nan
Copy link
Member

@B4nan B4nan commented Jan 15, 2021

Sometimes we might want to disable identity map and change set tracking for some query.
This is possible via disableIdentityMap option. Behind the scenes, it will create new
context, load the entities inside that, and clear it afterwards, so the main identity map
will stay clean.

const users = await orm.em.find(User, { email: '[email protected]' }, {
  disableIdentityMap: true,
  populate: { cars: { brand: true } },
});
users[0].name = 'changed';
await orm.em.flush(); // calling flush have no effect, as the entity is not managed

Keep in mind that this can also have
negative effect on the performance.

Closes #1267

Sometimes we might want to disable identity map and change set tracking for some query.
This is possible via `disableIdentityMap` option. Behind the scenes, it will create new
context, load the entities inside that, and clear it afterwards, so the main identity map
will stay clean.

```ts
const users = await orm.em.find(User, { email: '[email protected]' }, {
  disableIdentityMap: true,
  populate: { cars: { brand: true } },
});
users[0].name = 'changed';
await orm.em.flush(); // calling flush have no effect, as the entity is not managed
```

> Keep in mind that this can also have
> [negative effect on the performance](https://stackoverflow.com/questions/9259480/entity-framework-mergeoption-notracking-bad-performance).

Closes #1267
@B4nan B4nan merged commit 03da184 into master Jan 15, 2021
@B4nan B4nan deleted the disabled-id-map branch January 15, 2021 13:55
@dbettini
Copy link

dbettini commented Dec 16, 2021

@B4nan hello, I just found this issue while searching for a way to populate only specific relations on a model, without the possibility of loading more data as a consequence of using identity map.
What is the reason for using this.fork(false) instead of this.fork(true)? Looking at the docs, it seems like we don't get a clear identity map this way, but clone the existing one. In other words, the current implementation uses the existing identity map, it just doesn't update it, which will result in sometimes getting more data than required. I.e.:

const one = repo.findOne(id, ['firstRelation'])
const two = repo.findOne(id, { populate: ['secondRelation'], disableIdentityMap: true });

In the above example, two will also have firstRelation loaded, which would not be expected by the naming of disableIdentityMap.
Would you consider changing this behavior? Or, if not, is there any other way to accomplish this, other than manual forking?

@B4nan
Copy link
Member Author

B4nan commented Dec 16, 2021

I though it was required for the population to work, but looks like the tests are passing without clearning too. I am fine with the change in v5, for v4 it seems semi breaking.

@dbettini
Copy link

Cool, I'll stick to manual forking until v5 then. Thanks for the quick reply!

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