feat: Fetch adapter support for context provided via adapterOptions
#66
Conversation
| one.connectTo(FetchAdapter); | ||
|
|
||
| expect(function() { | ||
| two.connectTo(FetchAdapter); |
There was a problem hiding this comment.
connectTo vs providing the adapter via configure so that the constructor assignment happens and we can do cleanup (i.e., stop())
jasonmit
force-pushed
the
u/jasonmit/fetch-context
branch
2 times, most recently
from
89d9c8c
to
0da3667
Compare
| this.assert('Fetch global not found.', nativeFetch); | ||
| const { context } = this.options; | ||
|
|
||
| this.assert('Fetch global not found.', context.fetch); |
There was a problem hiding this comment.
!!(context && context.fetch)
Assert only checks for false or undefined. Might wanna open that up to any falsey value.
| this.handleRequest({ | ||
| url, | ||
| method: options.method || 'GET', | ||
| headers: FetchUtils.serializeHeaders(options.headers), | ||
| body: options.body, | ||
| requestArguments: [url, options] | ||
| }); | ||
|
|
||
| defineProperty(context.fetch, STUB_META, { value: {} }); |
There was a problem hiding this comment.
Why a meta object vs a Boolean?
| @@ -21,3 +21,36 @@ describe('Integration | Fetch Adapter', function() { | |||
| adapterTests(); | |||
| adapterBrowserTests(); | |||
| }); | |||
|
|
|||
| describe('Integration | Fetch Adapter | Concurrency', function() { | |||
There was a problem hiding this comment.
Can you also add a test against a null context and one without fetch?
There was a problem hiding this comment.
Yup need to add tests in general around context. Will finish up tonight + docs
I just meant the logic around checking the STUB_META instead of checking against the captured native class in the module. That way, we can still wrap a pre modified XHR. |
I'll tackle in a separate PR tomorrow since I want to chat first. |
jasonmit
force-pushed
the
u/jasonmit/fetch-context
branch
2 times, most recently
from
83677b4
to
05dbcc0
Compare
jasonmit
force-pushed
the
u/jasonmit/fetch-context
branch
from
05dbcc0
to
8f43b9a
Compare
I'll update the docs also.
I can't apply the same treatment to the XHR adapter since
nisedoesn't enable providing a context object. I think we spoke about this already, but mentioning again.Resolves #56