Expose an HTTP-request browser client#35486
Conversation
|
Pinging @elastic/kibana-platform |
💔 Build Failed |
838b2b2 to
fff35e0
Compare
💔 Build Failed |
ab8fe91 to
32af0ba
Compare
💔 Build Failed |
💔 Build Failed |
There was a problem hiding this comment.
I didn't use it because when you call .mockReturnValue(createSetupContractMock()), typescript doesn't infer type to HttpService.setup yet.
you can pass .mockReturnValue({}), and it won't complain.
There was a problem hiding this comment.
I didn't use it
Could you elaborate on what you mean by this?
There was a problem hiding this comment.
that's why I prefer next form:
// here we infer type manually
const startContract: StartContract = {
setup: jest.fn()
}
// now we have proper validation
startContract.mockReturnValue(..)There was a problem hiding this comment.
This is currently inconsistent with the way the rest of the client-side mocks work, so I'm going to leave the current form. We can re-evaluate this again in the future.
There was a problem hiding this comment.
Ah, now I know why you did it this way, @restrry. I think having the type-safety is preferable and we should probably change the ones I had changed back.
a02bda7 to
0e86ce1
Compare
|
@mistic, would you mind taking a look at this error? Additionally, we need to come up with a way for folks to be able to run the typechecks performed in the build task locally. Currently, it's isolated to the build. |
💔 Build Failed |
|
According to @spalger this was due to |
💔 Build Failed |
|
During the build the typecheck we did against the built assets are basically |
💔 Build Failed |
6fe3a69 to
e7c48ff
Compare
💔 Build Failed |
|
retest |
💔 Build Failed |
e7c48ff to
b2e04a4
Compare
💔 Build Failed |
💔 Build Failed |
💚 Build Succeeded |
51b1883 to
e3db5be
Compare
💚 Build Succeeded |
* Expose an HTTP-request browser client * Fix failing tests from kfetch refactor * Make abort() non-enumerable, fix review issues * Move kfetch test setup to build-excluded location * Add ndjson tests to browser http service * Lint fixes * Fix missing update of del to delete in http mock * Fix problems with merging headers with undefined Content-Type * Delete correct property from updated options * Linting fix * Fix reference to kfetch_test_setup due to moving test file * Add tests and fix implementation of abortables * Add missing http start mock contract, fix test in CI * Remove abortable promise functionality * Fix DELETE method handler, remove unnecessary promise wrapper
* Expose an HTTP-request browser client * Fix failing tests from kfetch refactor * Make abort() non-enumerable, fix review issues * Move kfetch test setup to build-excluded location * Add ndjson tests to browser http service * Lint fixes * Fix missing update of del to delete in http mock * Fix problems with merging headers with undefined Content-Type * Delete correct property from updated options * Linting fix * Fix reference to kfetch_test_setup due to moving test file * Add tests and fix implementation of abortables * Add missing http start mock contract, fix test in CI * Remove abortable promise functionality * Fix DELETE method handler, remove unnecessary promise wrapper
💔 Build Failed |
💔 Build Failed |
Summary
This patch exposes a browser client for simplifying HTTP requests. The long-term goal of this service/client is to replace the legacy
kfetchservice. This patch also swaps out the internals ofkfetchfor this new service, meaning legacy and NP plugins are both using this.From a sat up
HttpService, there are now 5 additional methods:http.fetch(path, options): This has an API comparable towindow.fetchwith the addition of a few custom options, status code error handling, default HTTP header additions, and automatic body parsing resolution for JSON and NDJSON.http.get(path, options): TheGET-enforced shorthand forhttp.fetch().http.post(path, options): ThePOST-enforced shorthand forhttp.fetch().http.put(path, options): ThePUT-enforced shorthand forhttp.fetch().http.delete(path, options): TheDELETE-enforced shorthand forhttp.fetch().http.patch(path, options): ThePATCH-enforced shorthand forhttp.fetch().Fixes #18855.
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.This was checked for cross-browser compatibility, including a check against IE11Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportThis was checked for keyboard-only and screenreader accessibilityFor maintainers
Dev Docs
The HTTP browser client now exposes methods for simplifying HTTP requests. The long-term goal of this service/client is to replace the legacy
kfetchservice. This also swaps out the internals ofkfetchfor this new service, meaning legacy and new platform plugins are both backed by this functionality.From a sat up
HttpService, there are now 5 additional methods:http.fetch(path, options): This has an API comparable towindow.fetchwith the addition of a few custom options, status code error handling, default HTTP header additions, and automatic body parsing resolution for JSON and NDJSON.http.get(path, options): TheGET-enforced shorthand forhttp.fetch().http.post(path, options): ThePOST-enforced shorthand forhttp.fetch().http.put(path, options): ThePUT-enforced shorthand forhttp.fetch().http.delete(path, options): TheDELETE-enforced shorthand forhttp.fetch().http.patch(path, options): ThePATCH-enforced shorthand forhttp.fetch().