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

Update CalendarViewContent closures to accept nil return values #269

Merged
merged 2 commits into from
Sep 19, 2023

Conversation

calda
Copy link
Member

@calda calda commented Sep 18, 2023

This PR updates the closures on CalendarViewContent to accept nil return values. This makes it easier for consumers to handle unexpected cases.

For example, today using [weak self] with CalendarViewContent is tricky:

CalendarViewContent(
  visibleDateRange: Date.distantPast...Date.distantFuture, 
  monthsLayout: .vertical)
  .dayItemProvider { [weak self] day in
    guard let self else { 
      // We have to return a non-optional value, but can't.
      // The only option is to crash.
      preconditionFailure("self unexpectedly deallocated already") 
    }
        
    return self.dayItemProvider(day)
  }

after these changes its much easier:

CalendarViewContent(
  visibleDateRange: Date.distantPast...Date.distantFuture, 
  monthsLayout: .vertical)
  .dayItemProvider { [weak self] day in
    guard let self else { return nil }
    return self.dayItemProvider(day)
  }

self.monthHeaderItemProvider = { [defaultMonthHeaderItemProvider] month in
guard let itemModel = monthHeaderItemProvider(month) else {
// If the caller returned nil, fall back to the default item provider
return defaultMonthHeaderItemProvider(month)
Copy link
Member Author

Choose a reason for hiding this comment

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

To avoid having to change any of the existing call sites, we can have the instance properties still always return a non-nil value, even if the user-provided closure returns nil. An easy way to do this is to just fall back to the existing default item provider, which always returns a valid value.

@bryankeller bryankeller self-requested a review September 19, 2023 03:29
Copy link
Contributor

@bryankeller bryankeller left a comment

Choose a reason for hiding this comment

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

Awesome, thanks @calda !

@bryankeller bryankeller merged commit fb92cdd into airbnb:master Sep 19, 2023
2 checks passed
@bryankeller bryankeller added the enhancement New feature or request label Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants