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

Fiber layout/reconciliation issues #523

Open
ahti opened this issue Oct 1, 2022 · 1 comment
Open

Fiber layout/reconciliation issues #523

ahti opened this issue Oct 1, 2022 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@ahti
Copy link
Contributor

ahti commented Oct 1, 2022

Sample code

import TokamakDOM
import Foundation

@main
struct TokamakApp: App {
    static let _configuration: _AppConfiguration = .init(reconciler: .fiber(useDynamicLayout: false))
    var body: some Scene { WindowGroup("Tokamak App") { ContentView() } }
}

enum State {
    case a
    case b([String])
    case c(String, [Int])
    case d(String, [Int], String)
}

final class StateManager: ObservableObject {
    private init() { }
    static let shared = StateManager()

    @Published var state = State.a
}

struct ContentView: View {
    @ObservedObject var sm = StateManager.shared

    var body: some View {
        switch sm.state {
        case .a:
            // VStack {
                Button("go to b") {
                    sm.state = .b(["eins", "zwei", "drei"])
                }
            // }
        case .b(let arr):
            VStack {
                Text("b:")
                ForEach(arr, id: \.self) { s in
                    Button(s) {
                        sm.state = .c(s, s == "zwei" ? [1, 2] : [1])
                    }
                }
            }
        case .c(let str, let ints):
            VStack {
                Text("c \(str)")
                    .font(.headline)
                Text("hello there")
                ForEach(ints, id: \.self) { i in
                    let d = "i = \(i)"
                    Button(d) {
                        sm.state = .d(str, ints, d)
                    }
                }
            }
        case .d(_, let ints, let other):
            VStack {
                Text("\(other)")
                Text("count \(ints.count)")
            }
        }
    }
}

Describe the bug
Using the fiber reconciler, the sample code above misbehaves in various ways:

To Reproduce
Steps to reproduce the behavior:

  1. Run the sample code
  2. Click "go to b"
  3. => The correct buttons for state b appear, but the text does not
  4. Click any button
  5. => The "i = number" buttons for state c appear, but some from state b are left over, still no text

Changing the configuration to use dynamic layout puts all the buttons on top of each other.

If you add the VStack to wrap the button for state a, and apply the change from #522 the buttons update properly, but the text is still missing.

Expected behavior
The way the code behaves when commenting out the _configuration. No lingering buttons, all texts show up.

Desktop:

  • macOS 12.6
  • Safari 16
  • Version of Tokamak: main branch
@ahti ahti added the bug Something isn't working label Oct 1, 2022
@ahti
Copy link
Contributor Author

ahti commented Oct 9, 2022

I did a little bit of digging, and I think the underlying issue here occurs when the view a fiber represents changes between one not being represented by an element (button) and one that should have an element (vstack). ReconcilePass.reconcile(...) doesn't return any mutations when the fiber element is nil, but the tree reducer doesn't create the element when updating an existing fiber, it only sets up newContent.

If I find some more time I'll have a go at actually trying to find a fix, but I don't really know the code base well yet so that might be a while.

@carson-katri carson-katri linked a pull request Nov 1, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

2 participants