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

Add support for preferences, @StateObject, @EnvironmentObject, and custom DynamicProperty types #501

Merged
merged 28 commits into from
Jul 5, 2022

Conversation

carson-katri
Copy link
Member

@carson-katri carson-katri commented Jun 29, 2022

The following are now supported in the Fiber reconciler:

  • Preferences
    • To support preferences keys, I made the ReconcilePass always start at the root of the view hierarchy (not just when dynamic layout is enabled), but it still only looks for changes once it reaches the first changed fiber.
  • @StateObject/@ObservedObject
  • @EnvironmentObject
  • @AppStorage/@SceneStorage
  • Custom DynamicProperty types

Currently, backgroundPreferenceValue does not correctly access the preference values. overlayPreferenceValue works because the overlay is evaluated after the underlying content. To fix backgroundPreferenceValue we could evaluate the background after the foreground and set the z-index to put it underneath.

A new internal API for testing the FiberReconciler using TestFiberRenderer was also added when creating tests for the new dynamic properties:

struct TestView: View {
  @State private var count = 0

  var body: some View {
    Button(action: { count += 1 }) {
      Text("\(count)")
        .identified(by: "count") // Use `identified(by:)` to mark views for later retrieval.
    }
    .identified(by: "increment")
  }
}

// Setup a reconciler for `TestView`.
let reconciler = TestFiberRenderer(.root, size: .zero).render(TestView())

// Get a proxy to a View with the id "count".
let count = reconciler.findView(id: "count", as: Text.self)
let increment = reconciler.findView(id: "increment", as: Button<Text>.self)

// Access `.view` to resolve the current content of the fiber.
XCTAssertEqual(count.view, Text("0"))

// Use dynamic member lookup to access properties.
increment.action?()

// Make sure the state changed. `view` is computed when accessed, so it can be used across reconcile passes (despite `current`/`alternate` swaps).
XCTAssertEqual(count.view, Text("1"))

@carson-katri carson-katri added the Fiber Changes related to the FiberReconciler or a FiberRenderer label Jun 29, 2022
@carson-katri
Copy link
Member Author

carton test builds correctly on my local machine, so not sure why it isn't excluding the Dispatch stuff on CI...

@carson-katri carson-katri requested a review from a team June 30, 2022 13:00
@@ -22,7 +22,7 @@ extension FiberReconciler.Fiber: CustomDebugStringConvertible {
{
return "Text(\"\(text.storage.rawText)\")"
}
return typeInfo?.name ?? "Unknown"
return "\(typeInfo?.name ?? "Unknown") \(ObjectIdentifier(self).debugDescription.split(separator: "(")[1].split(separator: ")")[0])"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we guarantee that ObjectIdentifier(self).debugDescription.split(separator: "(") will always return at least two elements? Maybe we need to guard for that?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just removed this from the description, since I was just using it to debug something. If you think I should keep the memory address though, using unsafeBitCast may be a better method?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not unless you found seeing memory addresses particularly useful during development 🙂

@MaxDesiatov
Copy link
Collaborator

Just had a super quick look, will review the rest of it by the end of this week.

Copy link
Collaborator

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

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

Overall seems legit, just some nits with requests for doc comments.

@carson-katri carson-katri requested a review from a team July 5, 2022 20:36
Copy link
Collaborator

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

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

Seems legit, thanks!

@carson-katri carson-katri merged commit 676760d into main Jul 5, 2022
@carson-katri carson-katri deleted the fiber/dynamic-properties branch July 5, 2022 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fiber Changes related to the FiberReconciler or a FiberRenderer
Development

Successfully merging this pull request may close these issues.

2 participants