-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Rebase tchack use fetch #5900
Rebase tchack use fetch #5900
Conversation
failure is |
NullVoxPopuli#1 (targeting your branch @NullVoxPopuli) fixes the yarn.lock issues and the lint error. Also, you might be interested by this PR (also targeting your branch @NullVoxPopuli): NullVoxPopuli#2. |
Both merged! |
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.
Hopefully this is the very last review, I gave this a last look through and noticed that in rest-adapter (which json-api adapter extends) we import two utils from ember-fetch
import serializeQueryParams from 'ember-fetch/utils/serialize-query-params';
import determineBodyPromise from 'ember-fetch/utils/determine-body-promise';
Effectively this means that users must include ember-fetch
in their dependencies even if they use jQuery. I believe this just means we need a minor tweak to the dependencies check we already do.
The other option is to port those two utils into ember-data to avoid needing to force folks to install ember-fetch if they don't want/need it.
@rwjblue do you have thoughts?
Indeed, two ember-fetch's modules are imported in rest (and then json-api) adapter: NullVoxPopuli#3 |
shakes fist at lock files |
d961d2c
to
83b13d1
Compare
move ember-fetch to a peer-dependency fix jsonapi adapter tests add tests for fetch
* Fix lint error * Fix `_requireBuildPackages` error * Reset yarn.lock from master There seems to be some issues in the yarn.lock To solve them I ran: - `git checkout master yarn.lock` - `yarn` Throw an error in case of dependency mismatch (#2) * Throw an error in case of dependency mismatch An app (or addon) that consumes ember-data should have: - either jquery - or ember-fetch version 6.0.0 or above Throw a build error if it's not the case. * Remove ember-fetch as a peerDependency Move ember-fetch to dependencies (#3) Indeed, two `ember-fetch`'s modules are imported in rest-adapter.
83b13d1
to
e7f7b87
Compare
This just rebases @tchak's work from here: #5386