entity
entity copied to clipboard
feat: require explicit query context specification
Why
This is a RFC.
The proposal is to require the queryContext
argument to reduce the likelihood of accidental omittance within a transaction block in application code.
At Expo, we've noticed sometimes a transaction will be started and the loader/mutator calls within it may accidentally not include the query context (which must be explicitly supplied as an arg to run within the transaction):
await viewerContext.runInTransactionAsync(async (queryContext) => {
const existingNote = await NoteEntity.loader(viewerContext) // note how queryContext was accidentally omitted but tsc didn't have an error
.enforcing()
.loadByIDAsync(args.id);
return await NoteEntity.updater(existingNote, queryContext)
.setField('title', args.note.title)
.setField('body', args.note.body)
.enforceUpdateAsync();
});
Closes ENG-11824.
How
The proposed change is to always require queryContext
. At Expo, we added a method to our ViewerContext
subclass to vend the default EntityQueryContext
(non-transactional) for our default database adaptor flavor. I added a similar method to TestViewerContext
within this repo to demonstrate the convenience accessor.
RFC
- Does this pattern make sense?
- Is the default query context convenience pattern appropriate?
Test Plan
yarn tsc