Skip to content

Commit

Permalink
remove unsafe early exit on performDataSourceChange
Browse files Browse the repository at this point in the history
Summary:
By making `-performUpdate` completely synchronous sometimes, it unearthed a bug with the early exit where multiple updates can happen at once. Specifically, the shortcut doesn't block updates from being started within `-didUpdateToObject`.

Lets test removing it, since it's happening in a pretty sensitive part of updates.

Differential Revision: D60773894

fbshipit-source-id: 721dffcb6d5353d12693fa873eb0f502bcdf4c02
  • Loading branch information
Maxime Ollivier authored and facebook-github-bot committed Aug 6, 2024
1 parent 420fa27 commit 5a93080
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 8 deletions.
2 changes: 2 additions & 0 deletions Source/IGListDiffKit/IGListExperiments.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ typedef NS_OPTIONS (NSInteger, IGListExperiment) {
IGListExperimentInvalidateLayoutForUpdates = 1 << 2,
/// Throw NSInternalInconsistencyException during an update
IGListExperimentThrowOnInconsistencyException = 1 << 3,
/// Remove the early exit so multiple updates can't happen at once
IGListExperimentRemoveDataSourceChangeEarlyExit = 1 << 4,
};

/**
Expand Down
4 changes: 3 additions & 1 deletion Source/IGListKit/IGListAdapterUpdater.m
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,9 @@ - (void)performDataSourceChange:(IGListDataSourceChangeBlock)block {
// Which means we need to cancel the current transaction, flatten the changes from
// both the current transtion and builder, and execute that new transaction.

if (!self.transaction && ![self.transactionBuilder hasChanges]) {
if (!self.transaction
&& ![self.transactionBuilder hasChanges]
&& !IGListExperimentEnabled(self.experiments, IGListExperimentRemoveDataSourceChangeEarlyExit)) {
// If nothing is going on, lets take a shortcut.
block();
return;
Expand Down
15 changes: 8 additions & 7 deletions Source/IGListKit/Internal/IGListUpdateTransactionBuilder.m
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,12 @@ - (void)addChangesFromBuilder:(IGListUpdateTransactionBuilder *)builder {
- (nullable id<IGListUpdateTransactable>)buildWithConfig:(IGListUpdateTransactationConfig)config
delegate:(nullable id<IGListAdapterUpdaterDelegate>)delegate
updater:(IGListAdapterUpdater *)updater {
IGListCollectionViewBlock collectionViewBlock = self.collectionViewBlock;
if (!collectionViewBlock) {
return nil;
}

switch (self.mode) {
case IGListUpdateTransactionBuilderModeBatchUpdate:
case IGListUpdateTransactionBuilderModeBatchUpdate: {
IGListCollectionViewBlock collectionViewBlock = self.collectionViewBlock;
if (!collectionViewBlock) {
return nil;
}
return [[IGListBatchUpdateTransaction alloc] initWithCollectionViewBlock:collectionViewBlock
updater:updater
delegate:delegate
Expand All @@ -157,9 +156,11 @@ - (void)addChangesFromBuilder:(IGListUpdateTransactionBuilder *)builder {
applySectionDataBlock:self.applySectionDataBlock
itemUpdateBlocks:self.itemUpdateBlocks
completionBlocks:self.completionBlocks];
}
case IGListUpdateTransactionBuilderModeReload: {
IGListReloadUpdateBlock reloadBlock = self.reloadBlock;
if (!reloadBlock) {
IGListCollectionViewBlock collectionViewBlock = self.collectionViewBlock;
if (!reloadBlock || !collectionViewBlock) {
return nil;
}
return [[IGListReloadTransaction alloc] initWithCollectionViewBlock:collectionViewBlock
Expand Down

0 comments on commit 5a93080

Please sign in to comment.