Skip to content
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

Remove ember-fetch/mixins/adapter-fetch ember-fetch/ajax, drop node 8 #447

Merged
merged 4 commits into from
Mar 28, 2020

Conversation

stefanpenner
Copy link
Collaborator

@stefanpenner stefanpenner commented Feb 14, 2020

ember-fetch provides alot of additional functionality, that functionality comes at several costs:

  • size
  • complexity / maintenance

I would like us to deprecate, and stop providing these functionality. This functionality is:

  • ember-fetch/ajax: as jQuery is not longer supported, users should use fetch API’s
  • ember-fetch/errors: we recommend folks use HTTP status codes directly
  • ember-fetch/mixins/data: ember-data no longer needs this, it uses the fetch module provided by this add-on directly.

It allow users to utilize these features, we can point them to the 7.x branch, which we can maintain for a time. If users really want some of these features, other addons can be created, but having a kitchen sink of stuff here is unfortunately
For users who want these features, they can use 7.x branch

@stefanpenner stefanpenner changed the title Remove old compat Remove old compat stuff and only provide fetch module Feb 14, 2020
@nlfurniss
Copy link
Collaborator

One issue with removing the utils is that consuming apps then have to create all that logic themselves when making one-off fetch calls

@stefanpenner
Copy link
Collaborator Author

@nlfurniss which are the ones that you find very useful for apps? I’m open to keeping stuff around, if it really provides value. :)

@nlfurniss
Copy link
Collaborator

ember-fetch/utils/determine-body-promise
ember-fetch/utils/mung-options-for-fetch
ember-fetch/utils/serialize-query-params

I would argue that serialize-query-params is the least necessary, though it is convenient not to need to dupe this logic across multiple apps.

determine-body-promise is the most useful, as it makes dealing with the response very easy.

@stefanpenner
Copy link
Collaborator Author

stefanpenner commented Feb 14, 2020

@nlfurniss in your opinion does it make sense for those utilities to be in this repo, or as their own dependency? What % of fetch users do you believe do use or should use these utilities.

@xg-wang
Copy link
Member

xg-wang commented Feb 14, 2020

Ajax

Those 3 util functions are helpful for existing Ajax style requests, so their existence depends on the migration path we recommend for people using Ajax.
I think we should use https://github.com/ember-cli/ember-cli-version-checker#hassingleimplementation, with existing check added in #292 it should be okay for other addons list ember-fetch as dependency, so if people find direct fetch isn't enough they can create their own wrapper.
Related: #398 #222

Error

I think they are handy, if you feel strongly about it not being a pure polyfill I'm okay removing it.

ember-data

+1 to removal as the deprecation has been added. However we need to patch the old major to fix #391
My first try was blocked at ember-cli/ember-compatibility-helpers#42, maybe I can do what runspired suggested to build a debug macro. But anyway that's not against removing it.

Only provide fetch module

Have you looked at related proposal to not even have a fetch module? #330

@stefanpenner
Copy link
Collaborator Author

@nlfurniss / @xg-wang it sounds like we can split this into two steps:

  • drop ember-data related mixins, as they are no longer needed for modern ember-data, but leave the utils + ajax helper. (land soon)
  • explore the future of a more "pure" fetch world (explore and maybe land someday)

Thoughts?

stefanpenner and others added 2 commits March 27, 2020 23:00
* ember-fetch/mixinx: ember-data integrates fetch support since 3.10
* ember-fetch/ajax: as jQuery is not longer supported, users should use fetch API’s
@xg-wang
Copy link
Member

xg-wang commented Mar 28, 2020

Talked offline we're going to drop mixin + bump node in v8 major bump and explore the pure fetch in the future.

@xg-wang xg-wang changed the title Remove old compat stuff and only provide fetch module Remove ember-fetch/mixins/adapter-fetch ember-fetch/ajax, drop node 8 Mar 28, 2020
@xg-wang xg-wang merged commit 36c3a2b into master Mar 28, 2020
@xg-wang xg-wang deleted the simplify branch March 28, 2020 20:42
@bmaehr
Copy link

bmaehr commented Apr 29, 2020

@xg-wang If ajax is not supported anymore (I just installed this module because of this and I'm really annoyed about this 'We remove jquery everywhere but have no alternative for doing ajax request") then you shuild remove it from the Q&A of the README (main page of the project).

@xg-wang
Copy link
Member

xg-wang commented May 7, 2020

@bmaehr ah thanks for pointing out! Done in #517

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants