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

De-duplicate web build code #231

Merged
merged 3 commits into from
Jan 16, 2025
Merged

De-duplicate web build code #231

merged 3 commits into from
Jan 16, 2025

Conversation

TimJentzsch
Copy link
Collaborator

@TimJentzsch TimJentzsch commented Jan 16, 2025

Objective

Closes #229.

The bevy run web command cannot just wrap cargo run, as we don't want to run a native application, but run the code in the user's browser.
Hence, we have to manually build with cargo build instead.

This causes quite a lot of code (also including calling wasm-bindgen) to be duplicated between bevy build web and bevy run web.

To ensure consistency and aid development, the code should be shared where possible.
This will enable us to build more complex features like #226.

Solution

  • Extract reusable part of bevy build web into dedicated function.
  • Add a conversion from run args to build args.
  • Call the bevy build web logic in bevy run web as well.

The functionality should be equivalent.

In the future, we might refine how exactly the shared build function is called.
For example, I assume that we will pass the binary targets we want to build from the outside instead of determining it in the function itself, that will enable us to implement changes like #200 and #230.

@TimJentzsch TimJentzsch marked this pull request as ready for review January 16, 2025 20:07
@TimJentzsch TimJentzsch added C-Code-Quality An improvement of readability or quality A-Run Related to the bevy run command A-Build Related to the bevy build command A-Web Building or running Bevy apps targeting the browser D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Jan 16, 2025
Copy link
Contributor

@DaAlbrecht DaAlbrecht left a comment

Choose a reason for hiding this comment

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

Nice!, the duplicated logic was annoying me too ^^.

I tested bevy run web and bevy build weg both still work.

@TimJentzsch TimJentzsch merged commit c91a3a0 into main Jan 16, 2025
8 checks passed
@TimJentzsch TimJentzsch deleted the 229-deduplicate-build-logic branch January 16, 2025 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Build Related to the bevy build command A-Run Related to the bevy run command A-Web Building or running Bevy apps targeting the browser C-Code-Quality An improvement of readability or quality D-Straightforward Simple bug fixes and API improvements, docs, test and examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deduplicate build logic for web commands
2 participants