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 configuration options to App to choose reconciler #495

Merged
merged 14 commits into from
Jun 5, 2022
Merged

Conversation

carson-katri
Copy link
Member

@carson-katri carson-katri commented May 31, 2022

This PR enables support for App + Scene in the Fiber reconciler, and provides a way for apps to specify whether they want to use the FiberReconciler, and if they want to enable custom layout.

@main
struct MyApp: App {
  static let _configuration: _AppConfiguration = .init(
    reconciler: .fiber(useDynamicLayout: true)
  )
  
  var body: some Scene {
    WindowGroup {
      Text("Hello, world!")
    }
  }
}

@carson-katri carson-katri added the Fiber Changes related to the FiberReconciler or a FiberRenderer label May 31, 2022
@ezraberch
Copy link
Member

I'd prefer this interface:

  static let _configuration: _AppConfiguration = .init(
    useReconciler: .fiber,
    shouldLayout: true
  )

@carson-katri
Copy link
Member Author

I updated it to this:

static let _configuration: _AppConfiguration = .init(
  reconciler: .fiber(shouldLayout: true)
)

Reconciler options are:

.stack
.fiber()
.fiber(shouldLayout: true)

You can also pass the root environment in the configuration. Let me know what you think.

@carson-katri carson-katri changed the title Add useFiberReconciler configuration to App Add configuration options to App to choose reconciler May 31, 2022
@carson-katri
Copy link
Member Author

App should now properly handle @State.

@carson-katri carson-katri requested a review from a team June 4, 2022 13:13
Sources/TokamakCore/App/Scenes/Scene.swift Show resolved Hide resolved
Sources/TokamakCore/Fiber/App/AppVisitor.swift Outdated Show resolved Hide resolved
Sources/TokamakCore/Fiber/Fiber.swift Outdated Show resolved Hide resolved
Sources/TokamakCore/Fiber/Fiber.swift Show resolved Hide resolved
Sources/TokamakCore/Fiber/Fiber.swift Show resolved Hide resolved
Sources/TokamakCore/Fiber/FiberReconciler.swift Outdated Show resolved Hide resolved
Sources/TokamakDOM/App/App.swift Outdated Show resolved Hide resolved
case .stack:
_launch(app, configuration.rootEnvironment, TokamakDOM.body)
case let .fiber(useDynamicLayout):
// _ = Unmanaged.passRetained(
Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure if PR is ready for review, but if so, this commented code needs to be cleaned up

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.

Thank you for addressing all the nits! 😅 This looks great 👍

@carson-katri carson-katri merged commit 6e2ccf7 into main Jun 5, 2022
@carson-katri carson-katri deleted the fiber/app branch June 5, 2022 23:24
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.

3 participants