Improve SwiftPM support #1546
Closed
Improve SwiftPM support #1546
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: I was assuming the delegate would always be an IGListAdapterProxy, but this only happens if the client sets either the `collectionViewDelegate` or `scrollViewDelegate` properties on IGListAdapter. Otherwise the delegate will just be the IGListAdapter. Change the debug code to handle this case. We were starting to see some debug logs roll in with the delegate class listed as IGListAdapter. Hopefully this will return the correct class now. Differential Revision: D31690438 fbshipit-source-id: bf6dde57756fee4fe617944d45e152cae734b4f1
Summary: As title, currently https://instagram.github.io/IGListKit/ is not working. Let's try to rebuild the doc Reviewed By: Ziewvater Differential Revision: D31811862 fbshipit-source-id: 8a0b0ce6c73542da71dddd13c24b5eb9fe9b0506
…agram#1544) Summary: Pull Request resolved: Instagram#1544 Refactored and added functions to IGListAdapter for getting index path or scroll position of first visible item in list. This makes it possible to see which item is the current first visible item scrolled to in the list, and determine how far it is scrolled into (analogous to additionalOffset in scrollToObject:). Reviewed By: lorixx Differential Revision: D32196398 fbshipit-source-id: d809d5f96bb4e1d95055dbebe4e55c1a428a40ed
4 tasks
|
@lorixx has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@TimOliver has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
|
@TimOliver has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@TimOliver merged this pull request in c9e045c. |
|
Hi @cntrump! I'm so sorry for the amount of time it took for us to get back to you on this! Thanks a lot for the amazing PR! I've been playing with it internally and as a fork on my personal GitHub. I did have to modify it and roll back some of your changes though. As awesome as being able to configure SPM without having to use symlink references to every file is, it unfortunately didn't work when I tested it without using modular imports. I think this might be related to the fact that SPM is really strict on searching for headers outside of a target's directory, and we're relying on certain IGListDiffKit internal headers to build IGListKit. Hopefully as SPM improves, we can explore this again in the future. Thanks again! |
GreeMoz
pushed a commit
to appintheair/IGListKit
that referenced
this pull request
Jan 31, 2024
Summary: This diff imports and refines the PR made by cntrump on GitHub. The PR introduces the following: * Sample apps now use SPM instead of CocoaPods to import IGListKit. * Adds Mac Catalyst as an example target. * Adds C++ flags to the CocoaPods specs. * Fixes a script issue that was discovered when regenerating the symlinks. The PR originally aimed to remove the need for symlinked references to the IGListKit and IGListDiffKit source files, but in testing, I couldn't get it working. It's possible SPM being too strict [on where the headers can be placed to be discovered](https://forums.swift.org/t/how-do-i-specify-the-headers-directory-for-a-objc-target-in-swift-package-managers-package-swift/58531/3). Additionally, another issue was that the original PR changed all of the `#import` statements to the the modular `import` statements, which is fine for the sample apps, but ended up breaking compatibility for any apps that had modules disabled. ## Changes in this pull request Improve SwiftPM support: Build module `IGListDiffKit` and `IGListKit` as Objective-C++. module `IGListDiffKit`: - Source/IGListDiffKit - module defined in `Source/IGListDiffKit/modulemap/module.modulemap` - requires `-fmodules` and `-fcxx-modules` module `IGListKit`: - depend on `IGListDiffKit`, use `import IGListDiffKit;` - Source/IGListKit - module defined in `Source/IGListKit/modulemap/module.modulemap` - requires `-fmodules` and `-fcxx-modules` module `IGListSwiftKit`: - depend on `IGListKit`, use `import IGListKit` - Source/IGListSwiftKit Deleted `spm/` and `scripts/generate_spm_sources_layout.sh`, it is unnecessary. Updated `.podspec`, add `'OTHER_CFLAGS' => '-fmodules'` and `'OTHER_CPLUSPLUSFLAGS' => '-fcxx-modules'`. Add missing swift files for `IGListSwiftKit` in `IGListKit.xcodeproj` ### How to use Replace `#import <IGListDiffKit/IGListDiffKit.h>` with `import IGListDiffKit;`, because `IGListDiffKit.h` isn't exist in `modulemap/`, Or create a symbol link by `ln -sf ../IGListDiffKit.h` for support it ? ### Examples Use SwiftPM for building examples. ### Checklist - [x] All tests pass. Demo project builds and runs. - [x] I added tests, an experiment, or detailed why my change isn't tested. - [ ] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes. - [x] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md) Pull Request resolved: Instagram#1546 Test Plan: Test PR showing running tests: https://github.com/TimOliver/IGListKit/actions/runs/4339956050/jobs/7578047058 Reviewed By: lorixx Differential Revision: D33592395 Pulled By: TimOliver fbshipit-source-id: 8f7b1873f2b1c6a80908bb55b123e31bea13bb0c
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes in this pull request
Improve SwiftPM support:
Build module
IGListDiffKitandIGListKitas Objective-C++.module
IGListDiffKit:- Source/IGListDiffKit
- module defined in
Source/IGListDiffKit/modulemap/module.modulemap- requires
-fmodulesand-fcxx-modulesmodule
IGListKit:- depend on
IGListDiffKit, use@import IGListDiffKit;- Source/IGListKit
- module defined in
Source/IGListKit/modulemap/module.modulemap- requires
-fmodulesand-fcxx-modulesmodule
IGListSwiftKit:- depend on
IGListKit, useimport IGListKit- Source/IGListSwiftKit
Deleted
spm/andscripts/generate_spm_sources_layout.sh, it is unnecessary.Updated
.podspec, add'OTHER_CFLAGS' => '-fmodules'and'OTHER_CPLUSPLUSFLAGS' => '-fcxx-modules'.Add missing swift files for
IGListSwiftKitinIGListKit.xcodeprojHow to use
Replace
#import <IGListDiffKit/IGListDiffKit.h>with@import IGListDiffKit;, becauseIGListDiffKit.hisn't exist inmodulemap/, Or create a symbol link byln -sf ../IGListDiffKit.hfor support it ?Examples
Use SwiftPM for building examples.
Checklist
CHANGELOG.mdfor any breaking changes, enhancements, or bug fixes.