UI: Replace jQuery with fetch within Ember Data #8035
Conversation
Ember Asset Size actionAs of 06720bd Files that got Smaller 🎉:
Files that stayed the same size 🤷:
|
There was a problem hiding this comment.
Looks good, the per-commit way to read was indeed helpful, thanks. I’m glad there are some tests that cover the AbortController aspects. I do wonder whether this is a situation where a brief exercise in the various supported browsers is worth doing; IE in particular worries me.
| @@ -133,7 +133,7 @@ module('Unit | Adapter | Job', function(hooks) { | |||
| await settled(); | |||
|
|
|||
| assert.notOk( | |||
| pretender.handledRequests.mapBy('requestHeaders').some(headers => headers['X-Nomad-Token']), | |||
| pretender.handledRequests.mapBy('requestHeaders').some(headers => headers['x-nomad-token']), | |||
There was a problem hiding this comment.
It’s unfortunate that this matters 😞 but oh well
|
I also had your concerns around
|
DingoEatingFuzz
force-pushed
the
f-ui/ember-fetch
branch
from
41ae4cc
to
d0e0a6e
Compare
Ember Test Audit comparison
|
Going off of the error message being "Forbidden" was brittle to begin with and no longer works with Fetch due to the error message coming from jquery underpinnings that were unobserved by Ember Data's attempted recreation.
Adding this settled makes this test pass now that Ember Data is using fetch instead of jquery. The test was presumably always incorrect but never flaked.
DingoEatingFuzz
force-pushed
the
f-ui/ember-fetch
branch
from
d0e0a6e
to
06720bd
Compare
This fixes all the test failures seen here apart from the job adapter ones: https://app.circleci.com/pipelines/github/hashicorp/nomad/14501/workflows/01177cd9-a2ae-4fff-94ef-6b109b778123/jobs/134238 This was last toggled in #8035 and it’s frustrating to have to change it back but I couldn’t find another way 😞
This fixes all the test failures seen here apart from the job adapter ones: https://app.circleci.com/pipelines/github/hashicorp/nomad/14501/workflows/01177cd9-a2ae-4fff-94ef-6b109b778123/jobs/134238 This was last toggled in #8035 and it’s frustrating to have to change it back but I couldn’t find another way 😞
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
This work is part of #7834
Switching out
jQuery.ajaxwithfetchis theoretically easy with Ember Data. They made a best effort to first hide all of jQuery as an implementation detail and second use fetch in a way that emulates how jQuery works.However, we do a lot of idiosyncratic things with Ember Data including overriding the occasional private API in order to support blocking queries. It's not surprising that there was some tweaking that needed to happen.
I tried to be methodical with each commit, so reading through this commit-by-commit is advised.
Size Impact
I wouldn't expect this to have too much of a size impact. jQuery isn't being removed and Ember Data's fetch flag is evaluated at runtime, so there is no dead code elimination that takes place as a result of this switch.
Performance Impact
Since Ember Data plays a role in nearly all acceptance tests, it's possible there will be a performance impact.