API improvements by yozhgoor · Pull Request #78 · rustminded/xtask-wasm

yozhgoor · 2026-04-05T01:20:37Z

Closes #77

…`default_dist_dir_release`

…e_current_dir`

src/dist.rs

examples/demo/xtask/src/main.rs

src/dev_server.rs

src/dist.rs

src/lib.rs

README.md

src/dist.rs

src/dev_server.rs

…iated functions

cecton · 2026-04-08T17:01:33Z

examples/demo/webapp/examples/run_example.rs

 }

-#[xtask_wasm::run_example(static_dir = "webapp/static", app_name = "web_app")]
+#[xtask_wasm::run_example(assets_dir = "webapp/assets", app_name = "web_app")]


Common conventions:

For files processed/packed by webpack (imported from JS/CSS): put them under src/assets (or src/images, src/styles, etc.).

For static files served as-is (no bundling): put them in public or static (e.g., public/, static/ or public/assets/).

Most common names: "src/assets" for bundled assets and "public" for static assets. Recommendation: use src/assets for anything you import into code and public/ for favicons/index.html or truly static files.

We should go for public then. Not sure if we should change the name of the function (assets_dir) or nott.

Copilot

Pull request overview

This PR addresses issue #77 by refining the xtask-wasm public API to remove confusing boolean parameters, clarify dev-server configuration, and align naming (e.g., “assets” vs “static”), while updating documentation/examples accordingly.

Changes:

Renamed/reshaped key APIs: Dist::run → Dist::build, static_dir_path → assets_dir, dist_dir_path → dist_dir, and removed run_in_workspace.
Simplified dev-server usage: DevServer::xtask("dist").start() replaces arg("dist").start(default_dist_dir(false)).
Updated README, crate docs, and demo assets/examples to match the new API and add logger initialization.

Reviewed changes

Copilot reviewed 9 out of 16 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`src/dist.rs`	Introduces `build()` API, assets terminology, and new default dist-dir helpers; removes older APIs.
`src/dev_server.rs`	Adds `xtask()` and command pipeline (pre/main/post), changes `start()` to use an internal default dist dir.
`xtask-wasm-run-example/src/lib.rs`	Updates proc-macro args and generated CLI code to the new `assets_dir` / `xtask().start()` API.
`src/lib.rs`	Updates crate-level docs/examples to the new API and adds logger init in examples.
`README.md`	Updates the getting-started snippet to match the new API and logger initialization.
`examples/demo/xtask/src/main.rs`	Adapts demo xtask CLI to `assets_dir` + `build()` + `xtask().start()`.
`examples/demo/webapp/examples/run_example.rs`	Updates the proc-macro invocation to `assets_dir = ...`.
`examples/demo/webapp/assets/**`	Adds new asset files used by the updated demo (HTML + SASS/SCSS).
`src/wasm_opt.rs`	Minor error formatting change.
`Cargo.toml`	Bumps `rust-version` and adds `env_logger` dev-dependency.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-09T12:55:35Z

src/dev_server.rs

-            let handle = std::thread::spawn(|| match watch.run(command) {
-                Ok(()) => log::trace!("Starting to watch"),
-                Err(err) => log::error!("an error occurred when starting to watch: {}", err),
+    /// If `dist_dir` has not been provided, [`default_dist_dir_debug`] will be used.


The rustdoc link in this comment points to default_dist_dir_debug, but that symbol doesn’t exist anymore. Update it to reference the actual default (Dist::default_debug_dir()), or link to DevServer::dist_dir and describe the default path in plain text so the docs don’t contain a broken intra-doc link.

Suggested change

/// If `dist_dir` has not been provided, [`default_dist_dir_debug`] will be used.

/// If `dist_dir` has not been provided, [`Dist::default_debug_dir()`] will be used.

Copilot · 2026-04-09T12:55:36Z

src/dev_server.rs

+
+    /// Name of the xtask command that is executed when a change is detected.
+    ///
+    /// See [`command`] to use an arbitrary command.


In this rustdoc, See [command] is not an explicit intra-doc link to the command method and may resolve incorrectly or produce warnings. Prefer an explicit link like [command](Self::command) for consistency with the surrounding docs.

Suggested change

/// See [`command`] to use an arbitrary command.

/// See [`command`](Self::command) to use an arbitrary command.

Copilot · 2026-04-09T12:55:36Z

src/dev_server.rs

+    /// Adds an argument to pass to the [`command`](Self::command) executed when changes are
    /// detected.
    ///
-    /// This will use the xtask command by default.
+    /// This is no-op unless a [`command`](Self::command) or an [`xtask`](Self::xtask) is provided.
    pub fn arg<S: AsRef<ffi::OsStr>>(mut self, arg: S) -> Self {
-        self.set_xtask_command().arg(arg);
+        if let Some(command) = self.command.as_mut() {
+            command.arg(arg);
+        }
        self


arg/args/env/envs are now documented as a no-op unless command()/xtask() was called first. This is a silent behavioral change compared to the previous API (where calling arg() implicitly prepared an xtask command) and can lead to a dev server that watches/serves but never rebuilds without any error. Consider buffering these args/envs until a command is set, or returning an error when start() is called with pending args but no command configured.

Copilot · 2026-04-09T12:55:36Z

src/dist.rs

+        let assets_dir = if let Some(assets_dir) = self.assets_dir {
+            assets_dir
+        } else {
+            metadata.workspace_root.join("public").as_std_path().to_path_buf()
+        };


The docs say the default assets directory is public “at crate level”, but the implementation defaults to metadata.workspace_root.join("public"). In a workspace, this will look for public/ at the workspace root, not at the package/crate root, and may silently skip copying assets. Either adjust the docs to say “workspace root”, or compute the package directory from package_name (e.g., via crate::package(...)) and default to <package_root>/public.

Copilot · 2026-04-09T12:55:37Z

src/dist.rs

+/// at the workspace root, copy the content of the `project/assets` directory,
 /// generate JS bindings and output two files: `project.js` and `project.wasm`


This usage docs paragraph refers to copying project/assets, but the example crate is my-project above. Consider aligning the path in the prose with the rest of the example (my-project/assets) to avoid confusion.

Suggested change

/// at the workspace root, copy the content of the `project/assets` directory,

/// generate JS bindings and output two files: `project.js` and `project.wasm`

/// at the workspace root, copy the content of the `my-project/assets` directory,

/// generate JS bindings and output two files: `my-project.js` and `my-project.wasm`

Copilot · 2026-04-09T12:55:37Z

src/dist.rs

    /// Build the given package for Wasm.
    ///
    /// This will generate JS bindings via [`wasm-bindgen`](https://docs.rs/wasm-bindgen/latest/wasm_bindgen/)
-    /// and copy files from a given static directory if any to finally return
+    /// and copy files from a given assets directory if any to finally return
    /// the path of the generated artifacts.
    ///
-    /// Wasm optimizations can be achieved using [`crate::WasmOpt`] if the
-    /// feature `wasm-opt` is enabled.
-    pub fn run(self, package_name: &str) -> Result<PathBuf> {
+    #[cfg_attr(
+        feature = "wasm-opt",
+        doc = "Wasm optimizations can be achieved using [`WasmOpt`](crate::WasmOpt) if the feature `wasm-opt` is enabled."
+    )]
+    #[cfg_attr(
+        not(feature = "wasm-opt"),
+        doc = "Wasm optimizations can be achieved using `WasmOpt` if the feature `wasm-opt` is enabled."
+    )]
+    pub fn build(self, package_name: &str) -> Result<PathBuf> {
        log::trace!("Getting package's metadata");
        let metadata = metadata();


This is a breaking rename from run() to build() (and related builder renames/removals). If you intend to keep this compatible within the 0.5.x line, consider adding #[deprecated] wrapper methods (e.g., run delegating to build) so downstream crates get compile-time guidance instead of hard breakage.

Copilot · 2026-04-09T12:55:37Z

xtask-wasm-run-example/src/lib.rs

@@ -76,7 +76,7 @@ impl RunExample {

            match ident.to_string().as_str() {
                "index" => index = Some(expr),
-                "static_dir" => static_dir = Some(expr),
+                "assets_dir" => assets_dir = Some(expr),
                "app_name" => app_name = Some(expr),
                _ => return Err(parse::Error::new(ident.span(), "unrecognized argument")),
            }


The proc-macro attribute key was renamed from static_dir to assets_dir, which will break existing downstream uses. If compatibility is desired, consider accepting static_dir as a deprecated alias that maps to assets_dir, and emitting a clear compile-time warning/error message guiding users to the new name.

yozhgoor added 9 commits April 4, 2026 21:07

Clippy: fix indentation in doc comments

ca28128

Deprecate default_dist_dir in favor of default_dir_dir_debug and …

e103a8c

…`default_dist_dir_release`

Deprecate run_in_workspace in favor of use_workspace_root and `us…

b921f1d

…e_current_dir`

Add missing logger initialization in example

fcb54d5

Change command and arg/args implementation

43a5173

Remove deprecated functions

77cdd86

self-review

435d476

Update README.md

0be34c7

Fix CI

af672b2

yozhgoor force-pushed the api-improvements branch from 61551f6 to af672b2 Compare April 5, 2026 02:19

yozhgoor added 2 commits April 5, 2026 04:33

Clean up documentation

b870cb3

Fix intra doc links

b0a49ab

cecton reviewed Apr 5, 2026

View reviewed changes

src/dist.rs Outdated Show resolved Hide resolved

examples/demo/xtask/src/main.rs Outdated Show resolved Hide resolved

src/dev_server.rs Outdated Show resolved Hide resolved

src/dist.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

yozhgoor added 6 commits April 5, 2026 08:27

Remove run_in_workspace from Dist

56339f1

Change dist_dir_path into dist_dir

943c9fb

Add a xtask method to DevServer

7f6aea1

Change dist_dir_path into dist_dir in dist.rs too

e64f724

Rename Dist::run into Dist::build

982f040

Update README.md

1b6dbb9

cecton reviewed Apr 5, 2026

View reviewed changes

README.md Outdated Show resolved Hide resolved

src/dist.rs Outdated Show resolved Hide resolved

src/dev_server.rs Outdated Show resolved Hide resolved

src/dev_server.rs Show resolved Hide resolved

src/dev_server.rs Outdated Show resolved Hide resolved

yozhgoor added 8 commits April 7, 2026 09:02

Change static_dir_path into assets_dir

ef26784

Rename static directory into assets in examples/demo

f667a80

Improve module documentation for DevServer

04ea1f9

Move default_dist_dir_debug and default_dist_dir_release as assoc…

5832a26

…iated functions

Add the env and envs methods on DevServer

0cce0a1

Add pre and post methods on DevServer

b290c21

Clean up

60063ed

Self review

bcf09c8

cecton approved these changes Apr 8, 2026

View reviewed changes

cecton requested a review from Copilot April 9, 2026 12:47

Copilot started reviewing on behalf of cecton April 9, 2026 12:48 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

	/// If `dist_dir` has not been provided, [`default_dist_dir_debug`] will be used.
	/// If `dist_dir` has not been provided, [`Dist::default_debug_dir()`] will be used.

	/// See [`command`] to use an arbitrary command.
	/// See [`command`](Self::command) to use an arbitrary command.

		/// at the workspace root, copy the content of the `project/assets` directory,
		/// generate JS bindings and output two files: `project.js` and `project.wasm`

Conversation

yozhgoor commented Apr 5, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cecton Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

cecton Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants