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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Optional] Environment-based Routing #50

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Conversation

pteasima
Copy link
Collaborator

@pteasima pteasima commented Jan 25, 2024

@tillkruss you're getting this for free 馃帀馃啌馃帀, if you want it.

I was messing about in SwiftUI today, on my free time. Stumbled upon something I liked a decided to stress test it on your project.

As briefly mentioned before, SwiftUI @Environment is also a way of doing dependency injection (I use it in some of my recent codebases). The code above allows us to automatically inject multiple Routers into the environment with very little boilerplate. So every view gets to have its own Router type but its still possible to deeplink programmatically to pretty much any screen.

Notes:

  • Imho this is peak SwiftUI 馃榾, but it also uses some complex code (with weird syntax) internally. The global router was fine too, so I'll understand if you don't merge this. If needed, I could cleanup the "public" syntax which we'll realistically use (e.g. when defining previews or tests), internal syntax can stay weird 馃檪.
  • Having multiple routers avoids "domain leakage", i.e. a view operates on its router's path as opposed to some global router's moviesPath or settingsPath or even deeper structures. The code is more modular.
  • In this example switchToNewInstance is a little overengineered to show that the views can even be decoupled to some extent. We could have just used TabRouter and SettingsView.Router directly in MoviesView.

Its a draft PR for now because I still have to uncomment some Preview code and clean up. Take it or leave it. If Im doing the cleanup, I'll start tracking time, but it'll be a few minutes.

@tillkruss
Copy link
Collaborator

This is cool. I'm gonna take a closer look after I merge what I've been doing here: https://github.com/tillkruss/ruddarr/pull/46/files

@tillkruss tillkruss self-assigned this Jan 25, 2024
@tillkruss tillkruss added the on hold Needs more information label Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on hold Needs more information
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants