pointfreeco / swift-composable-architecture

A library for building applications in a consistent and understandable way, with composition, testing, and ergonomics in mind.
https://www.pointfree.co/collections/composable-architecture
MIT License
12.07k stars 1.41k forks source link

Using both Sharing Data and backward-compatible NavigationLink seems to create some strange behaviors. #3125

Closed MorningStarJ closed 2 months ago

MorningStarJ commented 2 months ago

Description

First of all, thanks to the TCA developers for their contribution to the SwiftUI development environment. It’s truly an amazing piece of work.

I’m not sure if this is an issue with TCA, SwiftUI, or my usage. Perhaps I’ve overlooked something? But using both Sharing Data and tree-based navigation with backward-compatible NavigationLink seems to create strange navigation behavior.

The first time navigation is triggered, it immediately automatically returns, but it works normally the second time.

If I use SwiftUI’s NavigationLink directly, the navigation issues do not occur.

Checklist

Expected behavior

Navigation works as expected without automatically returning.

Actual behavior

The first time navigation is triggered, it automatically returns, but it works normally the second time.

https://github.com/pointfreeco/swift-composable-architecture/assets/15356218/b5854937-54db-45e6-83bb-d118e77972a9

Steps to reproduce

I’ve marked three warnings in the demo and have determined which code causes the issue to be reproducible. TestNavigationLink.zip

The Composable Architecture version information

1.10.4

Destination operating system

iOS 17.5, iOS 17.4

Xcode version information

Version 15.4 (15F31d)

Swift Compiler version information

swift-driver version: 1.90.11.1 Apple Swift version 5.10 (swiftlang-5.10.0.13 clang-1500.3.9.4)
Target: arm64-apple-macosx14.0
mbrandonw commented 2 months ago

Hi @MorningStarJ, this is happening because you are constructing a fresh new store inside the body of a view, which is almost never a correct thing to do:

if store.isGuided {
  ContentView(store: StoreOf<ContentFeature>.init(initialState: .init(), reducer: {
    ContentFeature()
  }))
}

This means if the AppView ever re-renders it will cause a whole new store to be created, and the previous store will be discarded, which means all of state will be reset.

Typically you would integrate the domain of ContentFeature into the parent domain, in this case AppFeature, so that you can do:

if store.isGuided {
  ContentView(store: store.scope(state: \.content, action: \.content))
}

Or if you really do want a store that is disconnected from the app-level store, then you could hold onto a static store:

static let contentStore = StoreOf<ContentFeature>(initialState: .init()) {
  ContentFeature()
}

…

if store.isGuided {
  ContentView(store: Self.contentStore)
}

But I think most likely this is not what you want to do.

Since this is not an issue with the library I am going to convert it to a discussion. Feel free to continue the conversation over there.