Skip to content

CLI: add --renderer argument #4320

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

CLI: add --renderer argument #4320

wants to merge 5 commits into from
+160 −62

Conversation

nicoburns
Copy link
Contributor

@nicoburns nicoburns commented Jun 24, 2025 •
edited
Loading

Objectives

@nicoburns nicoburns force-pushed the cli-renderer-flag branch from 7d8c23e to f0c5203 Compare June 30, 2025 14:31
@nicoburns nicoburns added this to the 0.7.0 milestone Jun 30, 2025
@nicoburns nicoburns added native Related to dioxus-native cli Related to the dioxus-cli program labels Jun 30, 2025
@nicoburns nicoburns marked this pull request as ready for review June 30, 2025 15:25
@nicoburns nicoburns requested a review from a team as a code owner June 30, 2025 15:25
@nicoburns nicoburns changed the title Add --renderer argument to the CLI CLI: add --renderer argument; resolve features recursively. Jun 30, 2025
@nicoburns nicoburns force-pushed the cli-renderer-flag branch 4 times, most recently from 9b6e58e to 2a8d4c7 Compare June 30, 2025 19:34
@nicoburns
Copy link
Contributor Author

Not sure if the playwright test failure is spurious or caused by my CLI changes

@nicoburns nicoburns force-pushed the cli-renderer-flag branch from 2a8d4c7 to 5446e26 Compare June 30, 2025 21:50
@nicoburns
Copy link
Contributor Author

It was a problem with my changes. But now that's fixed I'm getting failed: Cargo build failed - no output location. Toggle tracing mode (press t) for more information. which seems like it probably isn't?

@nicoburns nicoburns force-pushed the cli-renderer-flag branch 2 times, most recently from 06a3f8c to bbc4c18 Compare July 1, 2025 07:05
@nicoburns nicoburns changed the title CLI: add --renderer argument; resolve features recursively. CLI: add --renderer argument Jul 1, 2025
@nicoburns
Copy link
Contributor Author

Ok, the recursive feature resolution was causing issues (build failures), so I've stripped that out for now. We can land that separately (probably not urgent?).

nicoburns added 2 commits July 1, 2025 22:22
@nicoburns
Add --renderer argument to the CLI
5905bce 
Signed-off-by: Nico Burns <nico@nicoburns.com>
@nicoburns
Make --platform native work
0f0826f 
Signed-off-by: Nico Burns <nico@nicoburns.com>
@nicoburns nicoburns force-pushed the cli-renderer-flag branch from bbc4c18 to 0f0826f Compare July 1, 2025 21:22
jkelleyrtp
jkelleyrtp reviewed Jul 1, 2025
View reviewed changes
Cargo.toml Outdated
@@ -441,7 +441,6 @@ openssl = { version = "0.10", features = ["vendored"] }
# To make most examples faster to compile, we split out assets and http-related stuff
# This trims off like 270 dependencies, leading to a significant speedup in compilation time
[features]
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather the default behavior of cargo run --example todomvc to not be a runtime error - is there any reason why we can't keep "desktop" as the defualt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No reason except that IIRC it was me that added it recently. And so I was removing it again to put it back how I found it...

nicoburns
nicoburns commented Jul 1, 2025
View reviewed changes
packages/cli/src/build/request.rs Outdated
Comment on lines 581 to 583
None => Platform::TARGET_PLATFORM.unwrap(),
// TODO: should we always have a default
// None => return Err(anyhow::anyhow!("No platform specified and no platform marked as default in Cargo.toml. Try specifying a platform with `--platform`").into()),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jkelleyrtp This was the only thing I wasn't sure about. Should we pick a default platform if none is specified at all? Either way I ought to either remove or reinstate the commented out code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkelleyrtp jkelleyrtp jkelleyrtp left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
cli Related to the dioxus-cli program native Related to dioxus-native
Projects
None yet
Milestone
0.7.0
Development

Successfully merging this pull request may close these issues.

Compiling dioxus-native apps with dx doesn't work unless the desktop feature is enabled
2 participants
@nicoburns @jkelleyrtp