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

Uses native implementation when available #43

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rafaelnobrekz
Copy link

@rafaelnobrekz rafaelnobrekz commented Apr 11, 2023

Checklist

  • All tests are passed.
  • Added tests.
  • Documented the code using Xcode markup.
  • Searched existing pull requests for ensure not duplicated.

Description

This change aims to use the native diffable data source implementation whenever possible (which at this point is 90% of times). This avoids some small API discrepancies and bugs that have been fixed.

Related Issue

#42

Motivation and Context

This library has always (wisely) aimed at closely mimicking the native API because of its temporary nature. Ideally we don't want to have to use backports. But while we can't target the API we need, they are invaluable to adopt newer tech. In the case of DiffableDataSources though, we must use the backport even for the latest OS versions, which is unfortunate. The "sister" library I met along with this one was IBPCollectionViewCompositionalLayout, and it very neatly and transparently uses the backport only when the native API is not available. This is the aim of this PR. To make the transition to fully native as seamless as possible. Just remove the shims when you can and you're good to go.

Impact on Existing Code

I have wrapped native API under the existing code, so as far as I can understand there is no impact. I simply carry a native instance when the OS allows, and skip the backport one. I introduced internal init methods that enable forcing the backport so the tests could focus on testing the backports instead of what's available on the simulator running the tests. The public API is still the exact same.

@@ -1,28 +1,71 @@
import UIKit
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this change also works on AppKit? 🤔

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that would make sense to import either UIKit/AppKit conditionally. As NSDiffableDataSourceSnapshot seems compatible with both.

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.

3 participants