[GitHub] williaster opened a new pull request #5772: [superset-client] add `SupersetClient`, refactor app

[GitBox <gi...@apache.org>]

williaster opened a new pull request #5772: [superset-client] add `SupersetClient`, refactor app
URL: https://github.com/apache/incubator-superset/pull/5772
 
 
   This PR implements [SIP-4](https://github.com/apache/incubator-superset/issues/5667), replacing all JS ajax calls with a new `SupersetClient` class that handles all async requests using `fetch` 🎉  
   
   It's a big PR due to the ajax refactoring, so here's an higher-altitude summary:
   
   #### `SupersetClient` usage*
   
   ```javascript
   // appSetup.js
   import SupersetClient from `@superset-ui/core`;
   
   SupersetClient.configure(...clientConfig);
   SupersetClient.init(); // CSRF auth, can also chain `.configure().init();
   
   // anotherFile.js
   import SupersetClient from `@superset-ui/core`;
   
   SupersetClient.post(...requestConfig)
     .then(({ request, json }) => ...)
     .catch((error) => ...);
   ```
   
   *I'm refactoring to this now
   
   #### `SupersetClient` handles:
   - `CSRF` token authentication 
       - it also queues requests in the case that another request is made before the token is received
       - it checks for a token before every request, an external app that uses this can detect this by catching calls
   - timeouts (not supported by default in fetch)
   - query aborts (also not supported by default in fetch, uses `AbortController` polyfill)
   - supports the following configuration options: 
       - `protocol = 'http'`
       - `host`
       - `headers`
       - `credentials = 'same-origin'` (`include` for non-Superset apps)
       - `mode = 'same-origin'`  (`cors` for non-Superset apps)
       - `timeout`
   - supports `get`s and `post`s. I considered adding `put`s/`delete`s but no existing ajax calls of this type are currently made
   - a given request supports the following options, which met the need of every async call in the app:
     - `url` or `endpoint`
     - `headers`
     - `body`
     - `timeout`
     - `signal` (for aborting, from `const { signal } = (new AbortController())`)
     - for posts
       - `postPayload` (key values are added to a `new FormData()`)
       - `stringify` whether to call `JSON.stringify` on `postPayload` values
   
   #### New dependencies
   It introduces the following polyfills:
   - `AbortController`
   - `whatwg-fetch`
   - `mockFetch` for testing
   
   #### How was this tested?
   - attempted to test success + failure of every endpoint that was refactored in the browser
   - updated broken / ajax-containing tests
   - (TODO) add `SupersetClient` tests 
   
   #### Next steps
   0. finish todos
   1. merge this PR, test in staging, fix any immediate bugs
   2. move `packages/core` to `superset-ui` repo
   3. update imports to pull from `@superset-ui/core`
   
   #### TODO
   - [x] fix tests
   - [x] debug SQL lab polling
   - [x] remove unused `SupersetClient` methods like `put` and `destroy`
   - [ ] refactor API
   - [ ] rebase
   - [ ] add `SupersetClient` tests
   - [ ] verify sqllab error links
   
   @mistercrunch @kristw @conglei @graceguo-supercat @hughhhh @betodealmeida @michellethomas 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org