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

launch should not be in prelude and LaunchBuilder and associated configs need to be reworked #2965

Open
jkelleyrtp opened this issue Sep 16, 2024 · 0 comments · May be fixed by #2967
Open
Assignees
Labels
breaking This is a breaking change consistency improve cohesiveness of features
Milestone

Comments

@jkelleyrtp
Copy link
Member

jkelleyrtp commented Sep 16, 2024

Feature Request

The launch function for dioxus apps should not be in the prelude. Since this function is used only once in dioxus apps, it does not belong in the prelude which is intended as a utility support glob import.

To launch a dioxus app, you should go through the dioxus::launch function:

fn main() {
    dioxus::launch(app);
}

This is generally just "good practice" to only include the minimal set of functions/traits in the prelude, as well as making it possible to discover other, new functions by using intellisense. We can then make things like the LaunchBuilder accessible via the launch module:

dioxus::launch::builder()
    .with_cfg(|| MyConfig)
    .launch(app);

Clean up the launch crate:

The launch system we have now is supported by a bunch of macro_rules that make understanding launch quite confusing.

By default, we even mark launch as deprecated when no features are enabled, making launch even more confusing when using it with a default rust-analyzer setup.

This is also wired up into a build.rs and env!() calls that break proper caching, leading to very long incremental compile times.

The new design:

  • Should not lean into so many macro_rules
  • Should not be so typesafe (with_cfg can take any impl Config but not be connected to the renderer)
  • Should not export launch_desktop, launch_mobile, etc.
  • Should not specify Send/Sync of Context and instead take a Send/Sync callback that produces a !Send/!Sync Context
  • Should simply panic with a very pretty message if no platform is specified.
  • Should not be so generic (renderers should just attempt to downcast a named config)

Notes

I originally took a stab at this in #2779 but dropped those changes since they added 100+ changed files and should be in their own PR.

@jkelleyrtp jkelleyrtp added breaking This is a breaking change consistency improve cohesiveness of features labels Sep 16, 2024
@jkelleyrtp jkelleyrtp added this to the 0.6.0 milestone Sep 16, 2024
@jkelleyrtp jkelleyrtp changed the title launch should not be in prelude and LaunchBuilder and associated configs needs to be reworked launch should not be in prelude and LaunchBuilder and associated configs need to be reworked Sep 16, 2024
@ealmloff ealmloff linked a pull request Sep 16, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This is a breaking change consistency improve cohesiveness of features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants