-
-
Notifications
You must be signed in to change notification settings - Fork 616
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(exact): validate oneOf
chains
#1282
base: master
Are you sure you want to change the base?
feat(exact): validate oneOf
chains
#1282
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a possible solution to close the linked issue. I would like to work a little bit more on this one to make the code cleaner.
pushSubcontext(...contexts: ReadonlyContext[]) { | ||
this.subcontexts.push(...contexts); | ||
return this; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before merging this I want experiment with inheritance of contexts to see if I can create separate contexts for each of the middlewares we provide. All of them would conform to a shared interface, so that some methods are easily callable without type casting.
return; | ||
(internalReq[contextsKey] || []) | ||
.flatMap(context => { | ||
// TODO: This does not work for nested oneOf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is something I feel that inherited contexts may help to achieve
import { ContextItem } from '../context-items'; | ||
import { runAllChains } from '../utils'; | ||
|
||
// A dummy context item that gets added to surrogate contexts just to make them run | ||
const dummyItem: ContextItem = { async run() {} }; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this useful? Removing it did not make any test to fail.
Description
Closes #1269
To-do list