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

[MAJOR] Remove .ajax and allow async function for .setChoices #701

Merged
merged 8 commits into from
Oct 29, 2019
Merged

[MAJOR] Remove .ajax and allow async function for .setChoices #701

merged 8 commits into from
Oct 29, 2019

Conversation

tinovyatkin
Copy link
Contributor

@tinovyatkin tinovyatkin commented Oct 25, 2019

Removes .ajax method and introduces similar functionality by extending .setChoices

Description

Remove .ajax method due to bad name (it's not immediately clear what it does, and soon probably nobody will understand acronym, as I didn't remembered that AJAX stands for Asynchronous Javascript And XML 😳) and outdated functionality concept.

Introduces abiltiy to pass a callback or async function as first parameter to .setChoices to provide similar functionality for today' Promise world.

Motivation and Context

Almost all modern day fetching APIs are Promises based (all your examples except one are), while we are providing callback 😏 way to do that.

This also removes fetchFromObject helper function and delegates to consumer to do complete data transformation (fetchFromObject handles just one use case, without arrays, etc, and complete solution will require to introduce a huge dependency - so, it's better to let a consumer do whole transformation).

NOTE: This PR is not adding direct dependency on Promise implementation in Browser, so, not adding Promise to required polyfills.

How Has This Been Tested?

Refactored existing test and introduced new one.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@tinovyatkin
Copy link
Contributor Author

Just waiting for #700 to merge and rebase to remove that's code from here.

@tinovyatkin tinovyatkin marked this pull request as ready for review October 25, 2019 20:14
@tinovyatkin
Copy link
Contributor Author

This also closes #604

@tinovyatkin tinovyatkin changed the title [MAJOR] Remove .ajax, add .fetchChoices [MAJOR] Remove .ajax, add allow functions to .setChoices Oct 27, 2019
@tinovyatkin tinovyatkin changed the title [MAJOR] Remove .ajax, add allow functions to .setChoices [MAJOR] Remove .ajax and allow callback to .setChoices Oct 28, 2019
@tinovyatkin tinovyatkin changed the title [MAJOR] Remove .ajax and allow callback to .setChoices [MAJOR] Remove .ajax and allow async function for .setChoices Oct 28, 2019
@tinovyatkin
Copy link
Contributor Author

Also closes #626

@jshjohnson jshjohnson merged commit e3cc6ea into Choices-js:master Oct 29, 2019
@jshjohnson
Copy link
Collaborator

This is much cleaner - thanks!

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

Successfully merging this pull request may close these issues.

2 participants