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

ObservedPublisher for CurrentValueSubject #75

Closed
fatbobman opened this issue May 26, 2020 · 8 comments
Closed

ObservedPublisher for CurrentValueSubject #75

fatbobman opened this issue May 26, 2020 · 8 comments

Comments

@fatbobman
Copy link

ObservedPublisher is a great solution that combines @State and onRecevie into one. But when using CurrentValueSubject there is a problem, ObservedPublisher ignores dropping the first value.
I added a line to the code and now it adapts nicely to CurrentValueSubject.

self._subscription = .init(initialValue: publisher
       .delay(for: .nanoseconds(1), scheduler: RunLoop.main)
       .sink(receiveValue: {
            updateWrappedValue.value($0)
        }))
@vmanot
Copy link
Member

vmanot commented May 26, 2020

@fatbobman this is a great catch, and I actually faced the exact same problem while initially writing it!

Very neat workaround, just trying to think of any possible downsides. Could you also think about any potential problems with the delay? I'd be inclined to accept this solution and incorporate it immediately, except I've been burnt by optimistic reactive programming one too many times :p

@fatbobman
Copy link
Author

Okay. Since I'll be using this scheme for my own projects, I'll be watching closely and continuing to study it. The biggest pitfall at the moment is that the internal implementation mechanism of Combine is still unstable and may be subject to constant change.

@vmanot
Copy link
Member

vmanot commented May 28, 2020

@fatbobman how about this instead?

        self._subscription = .init(
            initialValue: Publishers.Concatenate(
                prefix: Just(initial)
                    .delay(for: .nanoseconds(1), scheduler: RunLoop.main),
                suffix: publisher
            ).sink(receiveValue: {
                updateWrappedValue.value($0)
            })
        )

The reason I'm hesitant to use the initial solution is because it's a constant delay applied to the publisher. I know, it's just one nanosecond, but it's the difference between synchronous and asynchronous - i.e. additional overhead.

@fatbobman
Copy link
Author

Good solution! The logic is also clearer.

@vmanot
Copy link
Member

vmanot commented May 29, 2020

@fatbobman could you see if this works for you, in your projects? I'm testing here as well.

@fatbobman
Copy link
Author

@vmanot I've tested it and it runs well in my project.

@vmanot
Copy link
Member

vmanot commented May 29, 2020

@fatbobman implemented. Thanks again!

@vmanot vmanot closed this as completed May 29, 2020
@JackYoustra
Copy link

Could you also use dropFirst?

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

No branches or pull requests

3 participants