Less statics#18
Conversation
WalkthroughThe PR refactors Tauri CLI's internal directory and configuration handling by replacing global static path resolution functions with an explicit Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.changes/reduce-internal-statics.md:
- Line 6: Update the phrase "user facing changes" in the
.changes/reduce-internal-statics.md diff to the hyphenated form "user-facing
changes"; locate the string "user facing changes" and replace it with
"user-facing changes" to correct the compound adjective grammar.
In `@crates/tauri-cli/src/add.rs`:
- Line 119: The install is being run in the wrong directory: PackageManager is
detected using frontend_dir but manager.install(&[npm_spec], dirs.tauri) uses
dirs.tauri as the working directory; change the second argument to the frontend
directory (frontend_dir) so PackageManager::install() runs npm in the frontend
project (refer to manager.install, PackageManager::install, frontend_dir and
dirs.tauri when updating the call).
In `@crates/tauri-cli/src/remove.rs`:
- Around line 47-50: The npm uninstall is being run from the wrong directory:
change the call to PackageManager::remove so it uses the frontend directory
(frontend_dir) as the working dir instead of dirs.tauri; specifically update the
manager.remove(&[npm_name], dirs.tauri)? invocation to pass the resolved
frontend_dir (or dirs.frontend) so PackageManager::remove runs the npm command
in the directory containing package.json.
🧹 Nitpick comments (4)
crates/tauri-cli/src/remove.rs (1)
21-29: Consider usingdirs.frontendinstead of callingresolve_frontend_dir()separately.
resolve_dirs()at line 21 already resolves and caches the frontend directory. You could useSome(dirs.frontend.clone())or adjust the logic to avoid the redundant resolution, thoughresolve_frontend_dir()returnsOption<PathBuf>which is needed for the conditional at line 47.crates/tauri-cli/src/mobile/mod.rs (1)
434-488: Avoid a duplicate PathBuf clone here.
You already materializetauri_dironce; reuse it to prevent an extra allocation.♻️ Suggested tweak
- let tauri_dir = tauri_dir.to_path_buf(); - App::from_raw(tauri_dir.to_path_buf(), raw) + let tauri_dir = tauri_dir.to_path_buf(); + App::from_raw(tauri_dir.clone(), raw)crates/tauri-cli/src/helpers/config.rs (1)
257-279: Consider updating theextensionsmap when merging configs.The
merge_config_withfunction updatesconfig.innerand setsTAURI_CONFIGenv var, but does not updateconfig.extensionsto reflect the merged config. This could lead tofind_bundle_identifier_overwriter()returning incorrect results if called aftermerge_config_with.If this is intentional (e.g.,
merge_config_withis only used in contexts whereextensionstracking isn't needed), consider adding a brief comment to clarify. Otherwise, consider updating the extensions map:💡 Suggested addition
merge(&mut value, &merge_config); config.inner = serde_json::from_value(value).context("failed to parse config")?; + config.extensions.insert(MERGE_CONFIG_EXTENSION_NAME.into(), merge_config); Ok(())crates/tauri-cli/src/mobile/android/build.rs (1)
120-132: Consider whether the Mutex wrapper is necessary here.The
commandfunction wrapstauri_configin aMutex, butrun()immediately locks it on line 154 and holds the lock for the entire function duration. If the config doesn't need concurrent access within this build flow, passingConfigMetadatadirectly (as done in other files likeandroid_studio_script.rs) would be simpler.The Mutex pattern may be intentional for consistency with other entry points that share the config across threads, but it adds unnecessary synchronization overhead in this synchronous build path.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
.changes/reduce-internal-statics.mdcrates/tauri-cli/src/acl/capability/new.rscrates/tauri-cli/src/acl/permission/ls.rscrates/tauri-cli/src/add.rscrates/tauri-cli/src/build.rscrates/tauri-cli/src/bundle.rscrates/tauri-cli/src/dev.rscrates/tauri-cli/src/helpers/app_paths.rscrates/tauri-cli/src/helpers/cargo_manifest.rscrates/tauri-cli/src/helpers/config.rscrates/tauri-cli/src/helpers/mod.rscrates/tauri-cli/src/icon.rscrates/tauri-cli/src/info/app.rscrates/tauri-cli/src/info/mod.rscrates/tauri-cli/src/info/plugins.rscrates/tauri-cli/src/inspect.rscrates/tauri-cli/src/interface/mod.rscrates/tauri-cli/src/interface/rust.rscrates/tauri-cli/src/interface/rust/desktop.rscrates/tauri-cli/src/interface/rust/manifest.rscrates/tauri-cli/src/migrate/migrations/v1/mod.rscrates/tauri-cli/src/migrate/migrations/v2_beta.rscrates/tauri-cli/src/migrate/mod.rscrates/tauri-cli/src/mobile/android/android_studio_script.rscrates/tauri-cli/src/mobile/android/build.rscrates/tauri-cli/src/mobile/android/dev.rscrates/tauri-cli/src/mobile/android/mod.rscrates/tauri-cli/src/mobile/android/run.rscrates/tauri-cli/src/mobile/init.rscrates/tauri-cli/src/mobile/ios/build.rscrates/tauri-cli/src/mobile/ios/dev.rscrates/tauri-cli/src/mobile/ios/mod.rscrates/tauri-cli/src/mobile/ios/run.rscrates/tauri-cli/src/mobile/ios/xcode_script.rscrates/tauri-cli/src/mobile/mod.rscrates/tauri-cli/src/remove.rs
🧰 Additional context used
🧬 Code graph analysis (21)
crates/tauri-cli/src/acl/permission/ls.rs (1)
crates/tauri-cli/src/helpers/app_paths.rs (1)
resolve_dirs(130-144)
crates/tauri-cli/src/info/app.rs (3)
crates/tauri-cli/src/info/plugins.rs (1)
items(107-139)crates/tauri-cli/src/info/packages_nodejs.rs (1)
items(133-151)crates/tauri-cli/src/info/packages_rust.rs (1)
items(12-67)
crates/tauri-cli/src/mobile/ios/run.rs (2)
crates/tauri-cli/src/helpers/config.rs (1)
get_config(239-245)crates/tauri-cli/src/mobile/android/run.rs (1)
options(84-88)
crates/tauri-cli/src/dev.rs (3)
crates/tauri-cli/src/helpers/config.rs (2)
get_config(239-245)reload_config(247-255)crates/tauri-cli/src/helpers/app_paths.rs (1)
resolve_dirs(130-144)crates/tauri-cli/src/interface/mod.rs (1)
new(112-112)
crates/tauri-cli/src/acl/capability/new.rs (1)
crates/tauri-cli/src/helpers/app_paths.rs (1)
resolve_dirs(130-144)
crates/tauri-cli/src/icon.rs (1)
crates/tauri-cli/src/helpers/app_paths.rs (1)
resolve_dirs(130-144)
crates/tauri-cli/src/helpers/cargo_manifest.rs (1)
crates/tauri-cli/src/interface/rust.rs (1)
get_workspace_dir(1282-1288)
crates/tauri-cli/src/interface/rust/desktop.rs (1)
crates/tauri-cli/src/interface/rust.rs (3)
out_dir(1202-1204)options(223-223)options(279-279)
crates/tauri-cli/src/mobile/android/mod.rs (1)
crates/tauri-cli/src/helpers/app_paths.rs (1)
resolve_dirs(130-144)
crates/tauri-cli/src/interface/mod.rs (1)
crates/tauri-cli/src/interface/rust.rs (10)
app_binary_path(934-959)options(223-223)options(279-279)get_binaries(961-1049)new(140-178)new(1081-1193)out_dir(1202-1204)target(1195-1200)get_cargo_metadata(1213-1231)build(184-193)
crates/tauri-cli/src/remove.rs (2)
crates/tauri-cli/src/helpers/app_paths.rs (2)
resolve_frontend_dir(146-167)resolve_dirs(130-144)crates/tauri-cli/src/helpers/npm.rs (1)
from_project(69-74)
crates/tauri-cli/src/helpers/app_paths.rs (1)
crates/tauri-utils/src/config/parse.rs (1)
into_file_name(43-49)
crates/tauri-cli/src/mobile/init.rs (4)
crates/tauri-cli/src/mobile/android/mod.rs (1)
get_config(127-191)crates/tauri-cli/src/mobile/ios/mod.rs (1)
get_config(124-284)crates/tauri-cli/src/mobile/mod.rs (4)
get_app(434-491)new(60-65)default(167-175)default(190-200)crates/tauri-cli/src/interface/mod.rs (1)
new(112-112)
crates/tauri-cli/src/mobile/ios/build.rs (3)
crates/tauri-cli/src/helpers/config.rs (1)
get_config(239-245)crates/tauri-cli/src/build.rs (2)
command(84-138)options(101-101)crates/tauri-cli/src/interface/mod.rs (2)
new(112-112)build(115-115)
crates/tauri-cli/src/mobile/mod.rs (2)
crates/tauri-cli/src/mobile/init.rs (1)
config(59-59)crates/tauri-cli/src/helpers/config.rs (1)
reload_config(247-255)
crates/tauri-cli/src/mobile/android/android_studio_script.rs (5)
crates/tauri-cli/src/helpers/config.rs (3)
get_config(239-245)reload_config(247-255)merge_config_with(258-279)crates/tauri-cli/src/helpers/app_paths.rs (1)
resolve_dirs(130-144)crates/tauri-cli/src/mobile/android/run.rs (1)
options(84-88)crates/tauri-cli/src/mobile/mod.rs (3)
read_options(395-432)get_app(434-491)new(60-65)crates/tauri-cli/src/interface/mod.rs (1)
new(112-112)
crates/tauri-cli/src/mobile/android/dev.rs (2)
crates/tauri-cli/src/helpers/config.rs (1)
get_config(239-245)crates/tauri-cli/src/helpers/app_paths.rs (1)
resolve_dirs(130-144)
crates/tauri-cli/src/mobile/android/build.rs (4)
crates/tauri-cli/src/helpers/config.rs (1)
get_config(239-245)crates/tauri-cli/src/helpers/app_paths.rs (1)
resolve_dirs(130-144)crates/tauri-cli/src/interface/mod.rs (2)
new(112-112)env(114-114)crates/tauri-cli/src/mobile/android/run.rs (1)
options(84-88)
crates/tauri-cli/src/add.rs (1)
crates/tauri-cli/src/helpers/app_paths.rs (2)
resolve_frontend_dir(146-167)resolve_dirs(130-144)
crates/tauri-cli/src/inspect.rs (3)
crates/tauri-cli/src/helpers/app_paths.rs (1)
resolve_dirs(130-144)crates/tauri-cli/src/helpers/config.rs (1)
get_config(239-245)crates/tauri-cli/src/interface/mod.rs (1)
new(112-112)
crates/tauri-cli/src/build.rs (8)
crates/tauri-cli/src/helpers/config.rs (1)
get_config(239-245)crates/tauri-cli/src/helpers/app_paths.rs (1)
resolve_dirs(130-144)crates/tauri-cli/src/interface/mod.rs (3)
new(112-112)env(114-114)app_settings(113-113)crates/tauri-cli/src/bundle.rs (2)
options(133-133)bundle(163-224)crates/tauri-cli/src/mobile/android/build.rs (1)
options(124-128)crates/tauri-cli/src/mobile/android/run.rs (1)
options(84-88)crates/tauri-cli/src/mobile/ios/run.rs (1)
options(103-103)crates/tauri-utils/src/lib.rs (1)
display_path(377-381)
🪛 LanguageTool
.changes/reduce-internal-statics.md
[grammar] ~6-~6: Use a hyphen to join words.
Context: ...config and directory resolvings, no user facing changes, please report any regres...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (115)
crates/tauri-cli/src/acl/permission/ls.rs (1)
26-33: LGTM! Clean refactoring to use explicitDirsstruct.The change correctly replaces the old
tauri_dir()global helper withresolve_dirs(), aligning with the PR's goal of removing static path resolution. The path construction foracl_manifests_pathremains functionally equivalent.crates/tauri-cli/src/icon.rs (1)
238-241: LGTM!The refactor correctly replaces the previous
tauri_dir()call with the newresolve_dirs()pattern, obtaining the Tauri directory from theDirsstruct and joining it with"icons"for the default output path.crates/tauri-cli/src/interface/rust/manifest.rs (1)
272-274: LGTM!The signature change to accept an explicit
tauri_dir: &Pathparameter aligns with the PR's goal of removing internal static path resolution. The manifest path is correctly computed from the provided directory.crates/tauri-cli/src/helpers/cargo_manifest.rs (1)
59-62: LGTM!The workspace directory lookup now correctly passes
tauri_dirtoget_workspace_dir, aligning with the updated function signature that requires explicit directory context.crates/tauri-cli/src/mobile/ios/run.rs (2)
78-106: LGTM!The refactor correctly:
- Resolves directories using
resolve_dirs()- Passes
&dirsto the build command- Obtains config using
get_tauri_configwithdirs.tauri- Wraps config in
Mutexfor thread-safe access in the watcherThis aligns with the pattern used in other platform-specific run commands.
131-139: LGTM!The watch invocation correctly passes both the mutex-wrapped config and dirs references, enabling config-driven watching behavior with the new directory-aware approach.
crates/tauri-cli/src/acl/capability/new.rs (2)
35-35: LGTM!The directory resolution correctly uses the new
resolve_dirs()function.
109-115: LGTM!The default capabilities path is correctly constructed from
dirs.tauri.join("capabilities"), aligning with the new directory-aware approach.crates/tauri-cli/src/info/plugins.rs (1)
114-136: LGTM! Clean refactor of plugin item generation.The restructured control flow is cleaner—gating on
tauri_dirfirst, then conditionally adding Node.js items whenfrontend_diris available. The earlycontinuepattern (lines 126-128) keeps the loop readable.crates/tauri-cli/src/info/mod.rs (1)
312-316: LGTM! Config-driven app item generation.The refactor properly gates app item population on successful config loading. Silently continuing when config fails is appropriate for an info command—other sections (environment, packages, plugins) still display useful information.
crates/tauri-cli/src/helpers/mod.rs (1)
74-86: LGTM! Clean dependency injection for hook working directory.Adding
frontend_diras an explicit parameter removes the hidden dependency on global state. The default working directory logic (line 86) remains correct—using the provided frontend directory whenscript_cwdisn't specified in the hook configuration.crates/tauri-cli/src/add.rs (2)
42-46: LGTM! Clean separation of command entry point and logic.Splitting
command()(resolves dirs) fromrun()(accepts dirs as parameter) enables reuse in migration contexts and improves testability.
145-148: LGTM! Consistent path construction for Rust entry points.Using
dirs.tauriforsrc/main.rsandsrc/lib.rspaths is correct—these files reside in the Tauri crate directory.crates/tauri-cli/src/mobile/android/run.rs (2)
81-91: LGTM! Proper config initialization and thread-safe sharing.Loading config once at command start and wrapping in
Mutexenables safe sharing with the watch callback. The config merge from CLI options (lines 84-88) is correctly applied before the mutex wrap.
155-163: LGTM! Config and dirs propagation to watch flow.Threading
&tauri_configand&dirsthrough tointerface.watchenables config-aware watching and consistent path handling during hot-reload cycles.crates/tauri-cli/src/mobile/android/mod.rs (1)
107-117: LGTM!The refactor correctly introduces
resolve_dirs()to obtain theDirsstruct and threads it through toinit_command. This aligns with the broader PR goal of replacing global static path resolution with explicit directory context.crates/tauri-cli/src/interface/rust/desktop.rs (3)
15-15: LGTM!Import correctly updated to include
Pathfor the newtauri_dir: &Pathparameter.
155-164: LGTM!The
build()function signature is correctly updated to accepttauri_dir: &Path, and theout_dirandapp_binary_pathcalls properly thread this parameter through. This aligns with the broader refactor to make path resolution explicit.
185-187: LGTM!The universal-apple-darwin branch correctly uses the same
tauri_dirparameter for per-tripleout_dircomputation, maintaining consistency with the main build path.crates/tauri-cli/src/migrate/mod.rs (3)
7-7: LGTM!Import correctly updated to include
crate_versionfromcargo_manifest, which is now used for version resolution.
17-30: LGTM!The migration command correctly:
- Resolves directories via
resolve_dirs()- Uses
dirs.taurifor manifest path construction- Passes
dirs.tauritoget_workspace_dirError messages properly reference the correct paths.
42-54: LGTM!The
crate_versioncall correctly receivesdirs.tauri, and both migration entry points (v1::runandv2_beta::run) properly receive&dirsas per their updated signatures.crates/tauri-cli/src/migrate/migrations/v2_beta.rs (2)
7-7: LGTM!Import correctly updated to include
Dirsfromapp_pathsmodule.
16-28: LGTM!The
run()function is correctly refactored to:
- Accept
dirs: &Dirsparameter- Derive
manifest_pathfromdirs.tauri- Pass
dirs.tauritomigrate_permissions- Pass
dirs.frontendtomigrate_npm_dependenciesThis provides better testability and aligns with the broader directory abstraction refactor.
crates/tauri-cli/src/mobile/ios/build.rs (5)
14-15: LGTM!Imports correctly updated to:
- Add
Dirsfor the new directory abstraction- Rename
get_configtoget_tauri_configfor clarity- Add
ConfigMetadatafor direct config access
170-207: LGTM!The
command()function is correctly refactored to:
- Accept
dirs: &Dirsparameter- Pass
dirs.tauritoget_tauri_config,AppInterface::new,get_app, andget_config- Use
dirs.tauriforset_current_dirThis properly threads the directory context through the iOS build flow.
216-238: LGTM!Resource injection and Info.plist handling correctly updated to:
- Pass
&tauri_configdirectly toinject_resources- Use
dirs.taurifor Info.plist path existence checks- Access
tauri_config.bundle.ios.info_plistdirectly
329-329: LGTM!The
run_buildcall correctly passes&dirsto thread directory context through the build execution.
344-383: LGTM!The
run_buildfunction is correctly refactored to:
- Accept
ConfigMetadatadirectly instead of a guarded handle- Accept
dirs: &Dirsfor directory context- Pass
dirstocrate::build::setup- Use
dirs.tauriforout_dircomputation- Pass
&tauri_configtowrite_optionsThis simplifies config access and maintains consistent directory handling.
crates/tauri-cli/src/info/app.rs (2)
6-8: LGTM!Import correctly updated to include
ConfigMetadataand simplify path imports to justPathBuf.
10-50: LGTM!The
items()function is cleanly refactored to:
- Accept
ConfigMetadatadirectly instead of loading config internally- Derive build-type, CSP,
frontendDist, anddevUrlfrom the config- Preserve framework/bundler inference from
package.jsonwhenfrontend_diris providedThis simplifies the function by removing the need for runtime config loading and aligns with the config-centric flow introduced in this PR.
crates/tauri-cli/src/inspect.rs (4)
4-4: Path import aligns with the new dir-aware signature.
25-28: Explicit dir resolution in the command flow looks good.
32-36: Config loading and AppInterface now correctly use the tauri_dir context.
47-52: Direct config access for WiX override is cleaner than the prior lock path.crates/tauri-cli/src/helpers/app_paths.rs (2)
26-29: Dirs container makes path context explicit and easy to thread.
130-143: resolve_dirs caches and returns both paths consistently.crates/tauri-cli/src/migrate/migrations/v1/mod.rs (1)
5-29: Dirs propagation through the v1 migration flow looks solid.crates/tauri-cli/src/mobile/init.rs (2)
7-68: Dirs is threaded cleanly through command/exec and config loading.
136-167: Platform config setup now consistently uses tauri_config refs (and dirs.tauri for iOS).crates/tauri-cli/src/mobile/ios/xcode_script.rs (3)
8-129: Dirs-based config loading/reload and AppInterface wiring is consistent.
140-148: In-place config merge after ensure_init is clear and contained.
236-284: Interface creation and out_dir resolution now use dirs.tauri consistently.crates/tauri-cli/src/mobile/mod.rs (4)
5-32: Imports updated appropriately for ConfigMetadata and Path usage.
217-219: Explicit handling for “no external IP” is clearer.
249-297: reload_config now threads tauri_dir through dev URL adjustments—nice.
495-516: ensure_init now reads identifier from ConfigMetadata directly—LGTM.crates/tauri-cli/src/mobile/android/android_studio_script.rs (6)
8-8: LGTM!Import aliases correctly updated to use the new function names (
get_config→get_tauri_config,reload_config→reload_tauri_config), maintaining consistency with the broader refactor while avoiding name collision with the localget_configfrom thesupermodule.
41-41: LGTM!Correct usage of the new
resolve_dirs()function to obtain theDirsstruct, replacing the previous global static path resolution pattern.
49-72: LGTM!The config initialization and reload flow is correctly refactored:
get_tauri_configreturnsConfigMetadatadirectly (no more Mutex wrapper)read_optionsaccepts&ConfigMetadatareferencereload_tauri_configtakes a mutable reference to update config in placedirs.tauriis correctly passed through the call chainThe pattern of conditionally reloading config when CLI options contain additional configs is preserved correctly.
65-78: LGTM!The
get_configandAppInterface::newcalls correctly use the new signatures with&tauri_config(direct reference) anddirs.taurifor path context. Theget_apphelper also receives the updated parameters.
88-96: LGTM!
merge_config_withcorrectly takes a mutable reference totauri_configand the config slice, aligning with the new API that mutates the config in place.
101-102: LGTM!Direct field access on
tauri_config.build.dev_urlis correct sinceConfigMetadataimplementsDeref<Target = Config>, eliminating the previous.lock().unwrap().as_ref().unwrap()chain.crates/tauri-cli/src/helpers/config.rs (6)
16-18: LGTM!Necessary imports added:
Pathfor the newtauri_dirparameter, andOnceLockfor the schema validator (correctly usingOnceLockper the MSRV constraint noted in the comment on line 143).
74-76: LGTM!Minor formatting adjustment for the
fips_compliantfield initialization using environment variable lookup.
152-157: LGTM!The
load_configfunction signature correctly updated to accepttauri_dir: &Pathas an explicit parameter, replacing the previous global state access. This enables directory-aware config loading throughout the codebase.
231-236: LGTM!
load_confignow returnsConfigMetadatadirectly viaOk(...)instead of storing it in a global static, completing the transition from Mutex-based global state to explicit value passing.
239-245: LGTM!
get_configis now a clean public API that delegates toload_configwithreload: false. The signature correctly requirestarget,merge_configs, andtauri_dirparameters.
247-255: LGTM!
reload_configcorrectly preserves thetargetfrom the existing config before reloading, then replaces the entire config in place. This maintains the target context across reloads.crates/tauri-cli/src/mobile/ios/mod.rs (5)
33-33: LGTM!Import correctly updated from
ConfigHandletoConfigMetadata, aligning with the refactored config handling approach.
105-116: LGTM!The command routing correctly obtains
dirsviaresolve_dirs()and passes&dirsto theInitandBuildsubcommands. TheDevandRuncommands don't receivedirshere, suggesting they resolve directories internally (which is consistent with their implementation patterns).
129-129: LGTM!The
get_configfunction signature correctly updated to accepttauri_dir: &Pathas the new parameter for directory-aware configuration.
547-556: LGTM!The
synchronize_project_configfunction correctly updated to accept&ConfigMetadatainstead of&ConfigHandle. The direct field access (tauri_config.identifier,tauri_config.product_name) works becauseConfigMetadataimplementsDeref<Target = Config>, eliminating the previous lock/unwrap pattern.
272-272: The review comment is based on an inaccurate premise and should be dismissed.The code at line 272 (
features: Some(ios_options.features)) is not a behavior change. TheCliOptions::featuresfield is defined asVec<String>(notOption<Vec<String>>), and consistently defaults toVec::new(). Both the iOS implementation (lines 229 and 272) and the Android implementation (lines 137 and 158) follow the identical pattern of always wrapping features inSome().The distinction between "no features specified" (
None) vs "empty features list" (Some(vec![])) cannot exist here becauseios_options.featuresis aVec, not anOption. If the field type wereOption<Vec<String>>, then the distinction would matter, but that's not the case in this codebase.Likely an incorrect or invalid review comment.
crates/tauri-cli/src/mobile/android/build.rs (7)
13-14: LGTM!Correct imports added for the new
Dirstype andConfigMetadatatype, along with the config getter function alias.
30-31: LGTM!Standard library imports added for
Path(used intauri_dirparameter) andMutex(used for config synchronization).
134-139: LGTM!The new
runfunction signature correctly acceptsdirs: &Dirsandtauri_config: &Mutex<ConfigMetadata>, enabling the build flow to be called from different entry points with varying config sources.
154-168: LGTM!The config is correctly locked and dereferenced. The subsequent calls to
AppInterface::new,get_app, andget_configcorrectly use the new signatures withdirs.tauriand direct config references.
176-191: LGTM!Directory context correctly propagated:
set_current_dirusesdirs.tauriensure_initreceivestauri_configreferencegenerate_tauri_propertiesreceivestauri_configreferencecrate::build::setupreceivesdirsreferenceAll calls align with the updated function signatures.
236-246: LGTM!The
run_buildfunction signature correctly updated to accepttauri_config: &ConfigMetadata(not Mutex, since lock is already held by caller) andtauri_dir: &Pathfor directory-aware operations.
261-275: LGTM!Internal calls correctly updated:
app_settings.out_dirreceivestauri_dirwrite_optionsreceivestauri_configreferenceinject_resourcesreceivestauri_configreferenceAll aligned with the new API signatures.
crates/tauri-cli/src/bundle.rs (6)
19-20: LGTM!Correct imports added for the new
Dirstype andConfigMetadatatype used throughout the bundling flow.
121-121: LGTM!
resolve_dirs()correctly called at the command entry point to obtain theDirsstruct for subsequent operations.
131-148: LGTM!The bundling setup correctly uses the new APIs:
get_configreceivesdirs.taurifor directory-aware config loadingAppInterface::newreceives config reference anddirs.tauriset_current_dirusesdirs.tauri- Direct field access on
config.bundle.macos.minimum_system_versionworks viaDerefapp_settings.out_dirreceivesdirs.tauriAll calls align with the refactored signatures.
150-159: LGTM!The
bundlefunction call correctly passes&config(direct reference) and&dirsfor the directory context, aligning with the updated function signature.
171-171: LGTM!The
bundlefunction signature correctly updated to acceptdirs: &Dirsas an additional parameter for directory-aware operations.
190-209: LGTM!Hook and settings calls correctly updated:
helpers::run_hookreceivesdirs.frontendfor thebeforeBundleCommandworking directoryapp_settings.get_bundler_settingsreceivesdirs.taurifor path context in binary/settings resolutionBoth calls align with the new parameter requirements.
crates/tauri-cli/src/interface/mod.rs (4)
12-18: LGTM!Imports correctly updated:
Mutexadded for thread-safe config access in dev/watch flowsDirs,Config, andConfigMetadataimported from helpers for the new parameter typesThe import organization is clean and brings in all necessary types.
32-46: LGTM!The
AppSettingstrait correctly updated withtauri_dir: &Pathparameters for:
get_bundle_settings- needs tauri_dir for resource path resolutionapp_binary_path- needs tauri_dir for output directory calculationget_binaries- needs tauri_dir for binary discoveryThese changes enable directory-aware operations throughout the build/bundle pipeline.
50-96: LGTM!The
get_bundler_settingsdefault implementation correctly propagatestauri_dir:
- Line 70:
self.get_binaries(&options, tauri_dir)- Line 78:
self.get_bundle_settings(&options, config, &enabled_features, tauri_dir)- Line 86:
rust::get_cargo_metadata(tauri_dir)for local tools directoryAll internal calls correctly receive the directory context.
109-136: LGTM!The
Interfacetrait correctly updated with the new parameter patterns:
newrequirestauri_dir: &Pathfor initializationbuildrequiresdirs: &Dirs(no config mutation needed)dev,mobile_dev,watchrequireconfig: &Mutex<ConfigMetadata>anddirs: &Dirsto support hot-reload scenarios where config may be mutatedThe distinction between
build(immutable config) anddev/watch(mutable config via Mutex) is intentional and appropriate for their use cases.crates/tauri-cli/src/dev.rs (5)
8-14: LGTM: Import changes align with the new directory abstraction.The updated imports correctly bring in
Dirs,get_config,reload_config, andConfigMetadatato support the new explicit directory context pattern.
99-136: LGTM: Clean threading ofDirsthrough the command flow.The refactoring correctly:
- Resolves directories once at the entry point (
command)- Passes
Dirsby value tocommand_internal(appropriate sinceDirsholds references)- Uses
dirs.taurifor config loading andAppInterfaceinitialization- Wraps config in
Mutexfor thread-safe access during dev workflow
138-148: LGTM: Updatedsetupsignature with explicit directory context.The function now accepts
&Dirsmaking the directory dependencies explicit. Using references is appropriate here sincesetupdoesn't need ownership.
144-160: LGTM: Correct path usage for mismatched packages check andbeforeDevCommand.
dirs.frontendis correctly used for the mismatched packages check (line 145) and as the default working directory forbeforeDevCommand(line 160)dirs.tauriis correctly used forset_current_dir(line 150)
264-268: LGTM: Config reload correctly usesdirs.tauri.The
reload_configcall properly passesdirs.taurifor resolving the configuration file path after the built-in dev server modifies the config.crates/tauri-cli/src/mobile/ios/dev.rs (6)
13-14: LGTM: Imports align with the new config and directory patterns.The aliased import
get_config as get_tauri_configavoids naming collision with the localget_configfrom the iOS module.
141-149: LGTM: Entry point correctly resolves and threadsDirs.Consistent with the pattern in
dev.rs- directories resolved once and passed through the call chain.
187-206: LGTM: Config and interface initialization correctly usedirs.tauri.The
get_tauri_config,AppInterface::new,get_app, andget_configcalls all correctly receivedirs.taurifor path resolution.
223-231: LGTM: Info.plist path resolution correctly usesdirs.tauri.All three plist file checks (
Info.plist,Info.ios.plist, and the config'sinfo_plist) properly usedirs.taurias the base path.
273-307: LGTM:run_devsignature and config handling.The function correctly:
- Takes
ConfigMetadatadirectly (not wrapped in Mutex initially)- Mutates config as needed before concurrent access
- Wraps in
Mutexat line 307 before passing to concurrent code paths
308-371: LGTM: Consistent threading ofdirsand locked config through mobile dev workflow.The
InterfaceOptions,mobile_dev, and closure callbacks correctly usedirs.taurifor path resolution and the mutex-locked config for concurrent access.crates/tauri-cli/src/build.rs (5)
10-11: LGTM: Build imports updated for directory abstraction.Clean import of
Dirsand the renamed config accessors.
84-137: LGTM: Build command correctly threadsDirsthrough the workflow.The build flow properly:
- Resolves directories once at entry
- Passes
dirs.taurito config loading and interface initialization- Passes
&dirstosetup,build, andbundleoperations- Uses
config.bundledirectly (no longer through a handle/lock)
140-158: LGTM: Setup function uses correct paths for each operation.
dirs.frontendanddirs.taurifor mismatched packages check (both are needed for comprehensive checking)dirs.tauriforset_current_dir
188-195: LGTM:beforeBuildCommandhook correctly usesdirs.frontend.The frontend directory is the appropriate working directory context for the before-build hook, consistent with how
beforeDevCommandis handled indev.rs.
221-223: LGTM: Cargo target directory resolution usesdirs.tauri.Correct since cargo metadata should be resolved from the Tauri project directory.
crates/tauri-cli/src/mobile/android/dev.rs (6)
13-14: LGTM: Android dev imports updated consistently.Same pattern as iOS dev -
DirsandConfigMetadatawith aliasedget_tauri_config.
134-142: LGTM: Entry point pattern matches other command flows.Consistent directory resolution and threading pattern.
187-200: LGTM: Config and interface initialization correctly usedirs.tauri.All path-dependent operations properly receive
dirs.tauri.
223-259: LGTM:run_devcorrectly handles config mutation and Mutex wrapping.Same pattern as iOS: mutate config before wrapping in Mutex for concurrent access.
268-273: LGTM: Out directory and tauri properties generation use correct paths.
out_dircorrectly usesdirs.tauri, andgenerate_tauri_propertiesreceives the locked config.
308-351: LGTM: Mobile dev callback and directory threading.The
mobile_devcall correctly threads the locked config anddirsthrough the mobile development workflow.crates/tauri-cli/src/interface/rust.rs (12)
34-35: LGTM: Interface imports updated for directory and config types.Clean addition of
Dirsand updated config type imports.
140-178: LGTM:Interface::newcorrectly accepts and usestauri_dir.The constructor properly:
- Uses
tauri_dirfor Cargo.toml watcher setup- Passes
tauri_dirtorewrite_manifest- Passes
tauri_dirtoRustAppSettings::new
184-237: LGTM:buildanddevmethods correctly threadDirs.Both methods accept
&Dirsand properly passdirs.taurito downstream functions.
240-288: LGTM:mobile_devandwatchmethods consistently updated.Both methods follow the same pattern of accepting config mutex and dirs references.
462-514: LGTM:get_watch_folderscorrectly usestauri_dir.The function properly:
- Gets workspace path from
tauri_dir- Uses
tauri_diras the base watch folder- Resolves relative additional watch folders from
tauri_dir
542-628: LGTM:run_dev_watchercorrectly handles config reload and path display.Key observations:
- Line 555:
get_watch_foldersusesdirs.tauri- Lines 596-598:
reload_configandrewrite_manifestusedirs.tauri- Line 609:
dirs.frontendused for user-friendly path display (stripping the frontend prefix from changed file paths)
853-932: LGTM:AppSettingstrait methods correctly accepttauri_dir.
get_bundle_settings, the bundler configuration, properly threadstauri_dirthrough totauri_config_to_bundle_settings.
934-1049: LGTM: Binary path resolution methods correctly usetauri_dir.
app_binary_pathandget_binariesproperly usetauri_dirfor:
- Resolving
out_dir- Looking up binaries in
src/bindirectory- Checking for
src/main.rs
1081-1205: LGTM:RustAppSettings::newandout_dircorrectly threadtauri_dir.The constructor uses
tauri_dirfor:
- Loading
CargoSettings- Getting workspace directory
- Loading
CargoConfigAnd
out_dircorrectly passestauri_dirtoget_target_dir.
1213-1288: LGTM: Cargo metadata and workspace functions correctly usetauri_dir.All cargo-related path resolution functions (
get_cargo_metadata,get_cargo_target_dir,get_target_dir,get_workspace_dir) consistently usetauri_diras the current directory context.
1306-1637: LGTM: Bundle settings correctly usetauri_dirfor path resolution.Key paths properly resolved from
tauri_dir:
- Line 1592:
Info.plistpath- Line 1634: License file path
1769-1848: LGTM: Tests updated to useresolve_dirs().The tests correctly:
- Call
resolve_dirs()to get theDirsstruct- Use
dirs.taurifor path-dependent test assertionsThis ensures tests validate the new API correctly.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| "tauri-cli": patch:changes | ||
| --- | ||
|
|
||
| Refactored internal use of static on config and directory resolvings, no user facing changes, please report any regressions if you encounter any |
There was a problem hiding this comment.
Minor grammar fix: hyphenate "user-facing".
When used as a compound adjective before a noun, "user-facing" should be hyphenated.
📝 Suggested fix
-Refactored internal use of static on config and directory resolvings, no user facing changes, please report any regressions if you encounter any
+Refactored internal use of static on config and directory resolvings, no user-facing changes, please report any regressions if you encounter any📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Refactored internal use of static on config and directory resolvings, no user facing changes, please report any regressions if you encounter any | |
| Refactored internal use of static on config and directory resolvings, no user-facing changes, please report any regressions if you encounter any |
🧰 Tools
🪛 LanguageTool
[grammar] ~6-~6: Use a hyphen to join words.
Context: ...config and directory resolvings, no user facing changes, please report any regres...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
In @.changes/reduce-internal-statics.md at line 6, Update the phrase "user
facing changes" in the .changes/reduce-internal-statics.md diff to the
hyphenated form "user-facing changes"; locate the string "user facing changes"
and replace it with "user-facing changes" to correct the compound adjective
grammar.
| _ => crate::error::bail!("Only one of --tag, --rev and --branch can be specified"), | ||
| }; | ||
| manager.install(&[npm_spec], tauri_dir)?; | ||
| manager.install(&[npm_spec], dirs.tauri)?; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the add.rs file around lines 97 and 119
head -130 crates/tauri-cli/src/add.rs | tail -40Repository: qodo-benchmark/tauri-combined-coderabbit
Length of output: 224
🏁 Script executed:
# Check PackageManager::install implementation
rg -n -B5 -A15 'pub fn install\(' crates/tauri-cli/src/helpers/npm.rsRepository: qodo-benchmark/tauri-combined-coderabbit
Length of output: 66
🏁 Script executed:
# Check how PackageManager is initialized and if cwd is set during creation
rg -n -B3 -A10 'PackageManager::new\|fn new\(' crates/tauri-cli/src/helpers/npm.rs | head -50Repository: qodo-benchmark/tauri-combined-coderabbit
Length of output: 66
🏁 Script executed:
# Look at remove.rs to compare the pattern
rg -n -B5 -A5 'manager\.install\|PackageManager' crates/tauri-cli/src/remove.rs | head -60Repository: qodo-benchmark/tauri-combined-coderabbit
Length of output: 66
🏁 Script executed:
cat -n crates/tauri-cli/src/add.rs | sed -n '85,130p'Repository: qodo-benchmark/tauri-combined-coderabbit
Length of output: 2024
🏁 Script executed:
cat -n crates/tauri-cli/src/helpers/npm.rs | sed -n '1,200p'Repository: qodo-benchmark/tauri-combined-coderabbit
Length of output: 6885
🏁 Script executed:
cat -n crates/tauri-cli/src/remove.rs | sed -n '85,135p'Repository: qodo-benchmark/tauri-combined-coderabbit
Length of output: 66
Fix NPM install directory mismatch.
The PackageManager is detected from frontend_dir (line 97), but manager.install() at line 119 is called with dirs.tauri. This will execute npm in the wrong directory. Since PackageManager::install() uses its second parameter as the working directory (via current_dir), npm will attempt to add the plugin to the Tauri crate directory instead of the frontend project where the dependency should be installed.
Pass the frontend directory to install() instead of dirs.tauri.
🤖 Prompt for AI Agents
In `@crates/tauri-cli/src/add.rs` at line 119, The install is being run in the
wrong directory: PackageManager is detected using frontend_dir but
manager.install(&[npm_spec], dirs.tauri) uses dirs.tauri as the working
directory; change the second argument to the frontend directory (frontend_dir)
so PackageManager::install() runs npm in the frontend project (refer to
manager.install, PackageManager::install, frontend_dir and dirs.tauri when
updating the call).
| if let Some(manager) = frontend_dir.map(PackageManager::from_project) { | ||
| let npm_name = format!("@tauri-apps/plugin-{plugin}"); | ||
| manager.remove(&[npm_name], tauri_dir)?; | ||
| manager.remove(&[npm_name], dirs.tauri)?; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the PackageManager implementation and the remove method signature
find . -type f -name "*.rs" | xargs grep -l "impl PackageManager" | head -5Repository: qodo-benchmark/tauri-combined-coderabbit
Length of output: 121
🏁 Script executed:
#!/bin/bash
# Search for the remove method in PackageManager
rg -A10 "fn remove\(" --type rust | grep -A10 "PackageManager" | head -30Repository: qodo-benchmark/tauri-combined-coderabbit
Length of output: 250
🏁 Script executed:
#!/bin/bash
# Check all usages of manager.remove to see the pattern
rg "manager\.remove" --type rust -B2 -A2Repository: qodo-benchmark/tauri-combined-coderabbit
Length of output: 614
🏁 Script executed:
#!/bin/bash
# Look at the context of the remove.rs file to understand dirs.tauri vs frontend_dir
cat -n crates/tauri-cli/src/remove.rs | head -60Repository: qodo-benchmark/tauri-combined-coderabbit
Length of output: 2127
🏁 Script executed:
#!/bin/bash
# Now look at the PackageManager implementation and remove method
cat -n crates/tauri-cli/src/helpers/npm.rs | head -100Repository: qodo-benchmark/tauri-combined-coderabbit
Length of output: 3648
🏁 Script executed:
#!/bin/bash
# Get more of the npm.rs file to see the remove method
wc -l crates/tauri-cli/src/helpers/npm.rsRepository: qodo-benchmark/tauri-combined-coderabbit
Length of output: 123
🏁 Script executed:
#!/bin/bash
# Search for the remove method in npm.rs
rg -n "fn remove" crates/tauri-cli/src/helpers/npm.rs -A15Repository: qodo-benchmark/tauri-combined-coderabbit
Length of output: 547
🏁 Script executed:
#!/bin/bash
# Get the full remove method implementation
sed -n '168,220p' crates/tauri-cli/src/helpers/npm.rsRepository: qodo-benchmark/tauri-combined-coderabbit
Length of output: 1381
🏁 Script executed:
#!/bin/bash
# Check what resolve_frontend_dir returns
rg -A10 "fn resolve_frontend_dir" crates/tauri-cli/src/helpers/app_paths.rsRepository: qodo-benchmark/tauri-combined-coderabbit
Length of output: 421
🏁 Script executed:
#!/bin/bash
# Check what resolve_dirs returns and what dirs.tauri is
rg -A20 "pub struct Dirs" crates/tauri-cli/src/helpers/app_paths.rs | head -30Repository: qodo-benchmark/tauri-combined-coderabbit
Length of output: 741
🏁 Script executed:
#!/bin/bash
# Let's verify the full resolve_dirs function to understand what dirs.tauri and dirs.frontend are
rg -A30 "pub fn resolve_dirs" crates/tauri-cli/src/helpers/app_paths.rsRepository: qodo-benchmark/tauri-combined-coderabbit
Length of output: 1205
Incorrect directory passed to npm package removal.
The PackageManager::remove() method (defined in npm.rs:168-172) expects the frontend directory as its second parameter and uses it as the working directory for the npm command. However, line 49 passes dirs.tauri (the Tauri project directory) instead of the frontend directory where package.json resides. This will cause the npm uninstall command to fail. Pass frontend_dir (already resolved on line 29) or dirs.frontend instead.
🤖 Prompt for AI Agents
In `@crates/tauri-cli/src/remove.rs` around lines 47 - 50, The npm uninstall is
being run from the wrong directory: change the call to PackageManager::remove so
it uses the frontend directory (frontend_dir) as the working dir instead of
dirs.tauri; specifically update the manager.remove(&[npm_name], dirs.tauri)?
invocation to pass the resolved frontend_dir (or dirs.frontend) so
PackageManager::remove runs the npm command in the directory containing
package.json.
1 participant
Benchmark PR from qodo-benchmark#142
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.