DioxusLabs / DioxusLabs/dioxus

dx components add fails to handle a rev for the registry properly

Open
#5,310 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Rust
Stars
39.1k
Forks
1.9k
Avg merge
4d 10h
Merged PRs (30d)
4

Description

**Problem**

I think there is a bug here, but I can't tease out what the intended fix should be.

The root of the bug seems to be that while `dx components add` allows `--git` and `--rev` to be specified, these do not apply to "built in" components, which always seem to be fetched from `main` of `https://github.com/DioxusLabs/components` (hard coded, not configurable).

The situation confuses `dx components add` such that it fails to add components like `date_picker`, which depend on other components and assets. It adds `date_picker` from the pinned rev I specify, and the things it depends on *without* that rev, and later errors out.

**Steps To Reproduce**

I pin dioxus-primitives like so in `Cargo.toml`:

```toml
dioxus-primitives = { git = "https://github.com/DioxusLabs/components", rev = "cc61648268e2d5a7291fb74276e1e7f6a12ae84e", default-features = false, features = ["router"] }
```

When I run `dioxus components add` I do it like so:

```sh
dx components add date_picker \
--git https://github.com/DioxusLabs/components \
--rev cc61648268e2d5a7291fb74276e1e7f6a12ae84e
```

My intent is to match my `dx components add` to the rev of dioxus-primitives, which lets me pull in components that were tested with that rev of dioxus-primitives. In other words, I don't want to be *forced* into upgrading just to add a new component. (I've actually got a whole vendor branch strategy that I'm trying to build on top of this workflow, where I advance the vendor branch by the specified rev and merge....)

**Expected behavior**

I expected `--git` and `--rev` to pin all components. That doesn't seem to be what happens. In `packages/cli/src/cli/component.rs` we have:

```rust
ComponentDependency::Builtin(name) => {
(ComponentRegistry::default(), name)
}
```

And in `packages/component-manifest/src/lib.rs` we have:

```rust
/// A dependency on another component, either built-in or third-party.
#[derive(Deserialize, Serialize, JsonSchema, Clone, Debug, PartialEq, Eq, Hash)]
#[serde(untagged)]
pub enum ComponentDependency {
Builtin(String),
ThirdParty {
name: String,
git: String,
#[serde(default)]
rev: Option,
},
}
```

In other words, it isn't even possible to express a "built in" dependency with a git rev, because `ComponentRegistry::default()` ends up here, and the default has no 'git' -- it hard codes to dioxuslabs/components.

```rust
/// If a git url is provided use that (plus optional rev)
/// Otherwise use the built-in registry
fn resolve_or_default(&self) -> (String, Option) {
if let Some(git) = &self.git {
(git.clone(), self.rev.clone())
} else {
("https://github.com/dioxuslabs/components".into(), None)
}
}
```

From here, things branch:

If this is intended behavior, then there is a bug described in https://github.com/DioxusLabs/dioxus/pull/5309 (draft PR). In short, in `copy_global_assets` there is code that tries to strip a common prefix, but the incomplete application of the rev pin causes assets to be pulled from a different registry root (the dirs in `~/.dx/cache/...`) and there is no common prefix to strip, so the command bails with an error.

If the Dioxus project would you'd like to support setting the git URL and rev of the "default" registry, then that'd be a different fix.

I could see *both* fixes being judged desirable, too.

**Environment:**

- Dioxus version: dioxus 0.7.3 (but code investigation was at current HEAD)

**Questionnaire**

I'm interested in fixing this myself but don't know where to start.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.