Skip to content

Conversation

@stephencelis
Copy link
Member

Combine's beta includes a new assign(to:) on publisher that can feed state updates to a @Published publisher. We don't want this, so let's ping objectWillChange directly.

https://developer.apple.com/documentation/combine/publisher/assign(to:)

@stephencelis stephencelis requested a review from mbrandonw June 25, 2020 16:16
Copy link
Member

@mbrandonw mbrandonw left a comment

Choose a reason for hiding this comment

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

sweet, thanks!

@stephencelis stephencelis merged commit 2ce84cc into main Jun 25, 2020
@stephencelis stephencelis deleted the avoid-published branch June 25, 2020 16:51
honghaoz pushed a commit to chouti-dev/swift-composable-architecture that referenced this pull request Jul 6, 2020
* Don't use @published for ViewStore state

* Update ci.yml

* Update format.yml
@DrewKiino
Copy link

DrewKiino commented Jul 8, 2020

I just updated my TCA to 0.6.0 and my SwiftUI ViewStore doesn't re-update it's state anymore. I did a small app with a @ObservableObject class and an inner variable with out @Published to mimic this new change and the view itself could not recognize the update until I added @Published back in. I think this breaks all iOS versions below 14.0.

How is the View suppose to update if @Published is removed from the inner state for ViewStore now? Really need this because we're using it for our production apps.

@stephencelis @mbrandonw

@mluisbrown
Copy link
Contributor

mluisbrown commented Jul 8, 2020

How is the View suppose to update if @Published is removed from the inner state for ViewStore now? Really need this because we're using it for our production apps.

All that @Published does is call self.objectWillChange.send() automatically before any change to the published variable belonging to a class conforming to ObservableObject so the change made in in this PR is just mimicking the functionality of @Published explicitly, therefore it should behave exactly as before.

I know this works since I had to do exactly the same thing in my ReactiveSwift version of the TCA.

@mbrandonw
Copy link
Member

@DrewKiino sorry to hear about that. All of the demo apps in the repo seem to work just as they did before.

And as @mluisbrown mentioned, we are just manually replicating what @Published gives to us for free, and we do that so that we can work around an escape each that Combine has introduced in iOS 14 allowing anyone from the outside to make changes to the state without sending an action.

Is it possible to share some code that would help us reproduce the problem you are seeing?

@DrewKiino
Copy link

Thanks for the quick reply 👍🏼 I was able to figure it out, it turns out that Xcode 11.4 doesn't support the new change for removing @Published and using objectWillChange, SwiftUI simply does not receive any updates. When I upgraded to Xcode 11.5, 0.6.0 started working for me so I think it was just a matter of Apple simply making a deprecated update that isn't as clear to most developers which was unfortunate.

https://www.hackingwithswift.com/quick-start/swiftui/how-to-send-state-updates-manually-using-objectwillchange

// quote from the above article
Note: SwiftUI actually provides us with a default objectWillChange property, but it doesn’t actually work in the current beta. So, it’s important to declare your own with the same name.

Which you can fix by adding this into your ObservableObject subclass

let objectWillChange = ObservableObjectPublisher()

Another article with the changes -
https://sarunw.com/posts/swiftui-changes-in-xcode-11-beta-5/

So basically, 0.6.0 requires Xcode 11.5 for the new changes to take effect due to Apple not making these changes backwards compatible.

@mbrandonw
Copy link
Member

Strange! I don't have Xcode 11.4 installed, but I do have Xcode 11.3 and all the case studies still seems to work, even without adding an objectWillChange to the ViewStore class. We want to support Xcode 11.3 and up, so if this is a problem I'd love to fix it. Can you describe how I might reproduce this locally?

@mluisbrown
Copy link
Contributor

@DrewKiino the posts you linked to are both referring to Xcode 11.0 betas from 2019, where this might have been a bug.

However in the ReactiveSwift version of ViewStore I do explicitly create the ObservableObjectPublisher, and I don't remember why I did that 🤷 . If I remove it all the Case Studies still work fine, although I am using Xcode 11.5.

@mluisbrown
Copy link
Contributor

I also just found this thread on the Swift Forums which seem to indicate that at least in November 2019, if you didn't have any @Published members, you had to instantiate your own ObservableObjectPublisher.

This comment by @broadwaylamb really makes this clear:

Since there's no @Published properties in your class, there's no storage objectWillChange could install the publisher into, so it just creates and returns a new instance every time you access objectWillChange.

I'm not sure when or if this behaviour changed, but IMO if you're not using any @Published properties, it would be safer to always instantiate your own ObservableObjectPublisher.

@broadwaylamb
Copy link

broadwaylamb commented Jul 9, 2020

@mluisbrown it has changed in the recent versions of Combine. It now falls back to a global lookup table if there are no @Published properties in the class.

@mluisbrown
Copy link
Contributor

Thanks for the really fast reply @broadwaylamb 🙏 Do you know in which version of Combine this changed, and when the change was made?

@broadwaylamb
Copy link

Unfortunately, no.

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.

6 participants