An Issue with methods of Rust struct and other V2.0 questions

[dbsxdbsx]

This is my 1st time using Frb v2.0(with dev. 32) in my own project since Frb v1.x.

1. An Issue with methods of Rust struct:
   If the struct is not defined in the Api folder, then its methods can't be generated
   --- This reminds me of the heavy time I spent on the multi-block issue PR to solve this issue(the potential recursive issue, because one of the fields of the issued rust struct could be another rust struct/enum, which may contain its own public methods...).
   (Given the extensive time and effort I have already dedicated to this issue, I am hesitant to consider further contributions in this area, especially V2.0 is still not stable and under active development. My latest v1.x modification without this issue could be referred here, though it has other bugs)

Questions on V2.0:

1. Within V2.0, for the tag rust_input in flutter_rust_bridge.yaml, how to specify more than one file respectively? (Seems no way at present)
   I know rust_input: rust/src/api/**/*.rs is supported, but how to specify more than one file respectively.
2. Within V2.0, now all the API fns can be directly invoked without a prefix, But I still suggest generating Dart Classes accordingly as in V1.x to make it more distinguishable.
   For example, in multi-block cases, with prefixes like Api1. Api2. can make it more clear in Dart side, and it can also be used to distinguish the same name dart methods---indeed, currently, it can be done by manually adding the dart class, but a little verbose, since it has to be done everytime the rust side fns is changed.
3. V2.0 introduces many macros, like #[frb(sync)],#[frb(ignore)], and compound usage case, I did find them in
   separated guidance file, but I think it is better to systematically introduce them in a specific page file in guidance in table style.

[fzyzcjy]

Hi!

If the struct is not defined in the Api folder, then its methods can't be generated

This looks like a bug, not a feature. Could you please create a minimal reproducible sample?

especially V2.0 is still not stable and under active development

Indeed it is almost stable, just put a .dev to allow rare breaking changes. The codebase will not have major overhaul.
But I totally understand your feeling when a huge PR cannot be merged though we both tried our best! :(

Within V2.0, for the tag rust_input in flutter_rust_bridge.yaml, how to specify more than one file respectively?

Could you please elaborate a bit about your scenario? The rust/src/api/**/*.rs syntax supports whole folders directly.

I still suggest generating Dart Classes accordingly as in V1.x to make it more distinguishable.

I am not very sure, but it seems that Dart's coding convention does not add the prefix. When users want the prefix, they can import 'some_file.dart' as some_prefix; some_prefix.SomeClass(). However, the proposal looks like a useful feature under some cases!

but I think it is better to systematically introduce them in a specific page file in guidance in table style.

Totally agree! I will do that in the next batch

[dbsxdbsx]

For the method issue, let me just use my real example(by the way, I didn't find the official multi-block example=_=):
the rust API is defined like this(the folder rust is at the root level):

in mod.rs, it is like this:

pub mod api_1;
pub mod api_2;

the lib.rs:

mod frb_generated; /* AUTO INJECTED BY flutter_rust_bridge. This line may not be accurate, and you can change it according to your needs. */
pub mod api;
mod database;
mod media;

use ctor::ctor;

#[ctor]
/// 这个函数会在库首次加载时被调用
fn called_on_load() {
    crate::init_logger("./logs/");
    log::info!("日志系统已初始化");
}
...

I guess using #[ctor] instead of #Frb[(init)], which has no side effect here, right?

, and in api_1.rs, the key 2 fns with customized struct MediaDoc is like this:

use crate::media::{CustomMediaCategory, MediaDoc};

#[tokio::main]
pub async fn init_media_docs(
    cms_list: CmsList,
    onlive_file_path: String,
    number: usize, // 所属分类要获取的总数，若category为None，则`number`代表所有总数
    target_media_type: Option<CustomMediaType>,
    media_name_to_search: Option<String>,
) -> Vec<MediaDoc> {
    if let Some(target_media_type) = target_media_type {
        if target_media_type.main_type == "直播" {
            return media::init_all_onlive_media_docs(&onlive_file_path, number).await;
        }
        media::init_all_cms_media_docs(cms_list, number, target_media_type)
            .await
            .unwrap_or_default()
    } else {
        return media::init_media_docs_by_name(cms_list, number, &media_name_to_search.unwrap())
            .await
            .unwrap_or_default();
    }
}

#[tokio::main]
pub async fn update_media_doc_detail(media_doc: MediaDoc) -> MediaDoc {
    media::update_media_doc_detail(media_doc)
        .await
        .unwrap_or_default()
}

again, the path tree is like this:

and the struct is defind like this:

#[derive(Default, Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct MediaDoc {
    pub names: Vec<String>,
  ...
}

impl MediaDoc {
    pub fn is_year_valid(&self) -> bool {
        self.year.chars().all(|c| c.is_ascii_digit()) && self.year.len() == 4
    }
    #[frb(sync)]
    pub fn get_tag_text(&self) -> String {
...
}

Could you please elaborate a bit about your scenario? The rust/src/api/**/*.rs syntax supports whole folders directly.

What I want is like before, specific to each file, like rust_input: rust/src/api/api_1.rs rust/src/api/api_2.rs. Anyway, this issue is trifle, I am ok with the folder basis mechanism.

When users want the prefix, they can import 'some_file.dart' as some_prefix; some_prefix.SomeClass(). However, the proposal looks like a useful feature in some cases!

Yes! I can import like import 'package:xiao_dong_tv/src/rust/api/api_1.dart' as api_1;, with this line, it is totally solved, and I guess there is no need for this feature.

By the way, I wonder what config flags dump_all, local, and full_dep exactly mean? I saw their explanation in pub(crate) struct GenerateCommandArgsPrimary {...}, but I am still at a loss, don't know when need to use them.

[fzyzcjy]

by the way, I didn't find the official multi-block example

Because the example shows a folder where there can have many files, so in v2 there is no speciality between single-block and multi-block.

let me just use my real example

Oh, IIRC we do not scan impl blocks outside the API folder? Then it seems that we should scan it.

it is totally solved, and I guess there is no need for this feature.

Happy to see it is solved!

By the way, I wonder what config flags dump_all, local, and full_dep exactly mean?

dump_all and local is for debugging (not for end users) - I probably should make the comments clearer. full_dep is because, by default I do not require users to install llvm (which ffigen requires). If you need some features that are missing without full_dep (and llvm), you can enable it; but normally this can be left as default.

[dbsxdbsx]

Thanks, I have no other questions. Feel free to close it when needed.

[fzyzcjy]

You are welcome. No worries, maybe we can leave it open and I will work on some of them a bit later.

[dbsxdbsx]

Hopefully, this issue could be fixed ASAP, because I realize it can't be even worked around:
For example, for the target struct named MediaDoc which both need fields and methods to be called in Dart, if putting the struct to the api_1.rs or api_2.rs, then it would raise the issue:

Error: Will generate duplicated class names (["MediaDoc"]). This is often because the type is auto inferred as both opaque and non-opaque. Try to 
add `#[frb(opaque)]` or `#[frb(non_opaque)]` to the struct, or change code that uses it.

By tagging with #[frb(non_opaque)], it has the same error msg. By tagging #[frb(opaque)], there is no error, but there is no field that can be called in Dart.

By the way, I also tried to put the struct in api/mod.rs(which I know is discouraged), then it had the same issue as putting the struct in the original non api folder.

In a word, I just can't find a workaround for dealing with a rust struct in which both fields and methods need to be called in Dart and used at least in one of the API files.

[fzyzcjy]

Hi, could you please provide a minimal reproducible sample? With your description I am not sure why it is both inferred as opaque and non-opaque... It should be reasonable to be inferred as non-opaque.

[dbsxdbsx]

Hi, could you please provide a minimal reproducible sample? With your description I am not sure why it is both inferred as opaque and non-opaque... It should be reasonable to be inferred as non-opaque.

Yes, exactly, I just found that not all struct would be failed, but with this one:

pub struct MyTestStruct {
    pub name: String,
    pub age: u32,
}
impl MyTestStruct {
    // comment out this function would make everything generated ok.
    // but with `pub` and `&mut self`, it would cause error.
    pub fn test_mut(&mut self) {
        self.name = "test_mut".to_string();
    }
}

IIRC, &mut self is supported due to some version of Frb v2.0.

[fzyzcjy]

I see: &mut self means that, it must be opaque. This is because non-opaque types are like "plain old java object (POJO)" (if you are familiar with that concept), and the object is created each and every time a function is called. Therefore, it is not possible to have a &mut, since it is not possible to modify the one on Dart side even if we do self.name = ....

By tagging #[frb(opaque)], there is no error, but there is no field that can be called in Dart.

I am thinking about another approach: What if we automatically generate getters and setters for opaque types? Like,

class MyTestStruct { // opaque type
  String get name => ..auto call rust..;
  set name(String val) => ..auto call rust..;
}

[dbsxdbsx]

I am thinking about another approach: What if we automatically generate getters and setters for opaque types? Like,

I think it could be a good idea. And I guess it means that the field needs to be called in Dart as methods like that in C#.

[fzyzcjy]

And I guess it means that the field needs to be called in Dart as methods like that in C#.

Not very sure, could you please elaborate a bit? In Dart we do have getters and setters, thus users can write myObject.myField and it indeed calls a real function String get myField => ...your implementation...

[dbsxdbsx]

Oh, so I misunderstood your idea. I am ok with both myObject.myField and myObject.myField()--the latter style is an encouraged way to interact with the field in a struct/class in some languages, like C# or even Rust.

I just realized there is a workaround at present--- Write Get/Set methods in Rust for all fields needed to be called in Dart, though it is quite tedious when there are many fields that need to be called in Dart.

[fzyzcjy]

the latter style is an encouraged way to interact with the field in a struct/class in some languages, like C# or even Rust.

I think so (Rust only supports the latter IIRC). On the other hand, it seems that Dart/Flutter encourages the former way, so I guess we can follow Dart's convension.

I just realized there is a workaround at present--- Write Get/Set methods in Rust for all fields needed to be called in Dart, though it is quite tedious when there are many fields that need to be called in Dart.

Totally agree it is tedious!

Talking about accessors, one big problem is that, it is good for things like String and int, but what if a field is of some complex type, or even some opaque type?

pub struct A {
  pub field b: B,
}

#[frb(opaque)]
pub struct B { }

Consider what is generated under the hood:

// impossible, since we do not support returning a reference (which will be troublesome for lifetime management)
fn a_field_b(a: &A) -> &B { a.b }

// is it ok? especially, sometimes it is not possible?
fn a_field_b(a: &A) -> B { a.b.clone() }

// seems also not good, because then the non-opaque `A` is consumed.
fn a_field_b(a: A) -> B { a.b }

[dbsxdbsx]

Talking about accessors, one big problem is that, it is good for things like String and int, but what if a field is of some complex type, or even some opaque type?

That is partly what I am concerned as I said at the very 1st post:

the potential recursive issue, because one of the fields of the issued rust struct could be another rust struct/enum, which may contain its own public methods...

And indeed as you said, this recursive issue could occur within params of methods of a rust struct/enum, but not only within fields.
In addition, what makes the design more complex is that Frb v2.0 is designed to support &mut and & param.

Consider what is generated under the hood:

From a user's perspective, as long as a method or function is valid in Rust, it should be supported for use in Dart. Frankly, designing this integration is beyond my capability and likely requires a global project context consideration to ensure it is constructive and easy to maintain for future Frb development.

[fzyzcjy]

Now auto getters/setters of opaque types is supported: https://cjycode.com/flutter_rust_bridge/guides/types/arbitrary/rust-auto-opaque/properties

And, opaque types can also be inside non-opaque types: https://cjycode.com/flutter_rust_bridge/guides/types/arbitrary/rust-auto-opaque/opaque-in-translatable

Thus closing this. Feel free to reopen if needed!

[dbsxdbsx]

@fzyzcjy, thanks your effort. Here is some feedbacks:

1. the fn new() issue:
   For such def in rust API file:

#[frb(opaque)]
pub struct MyTestStruct {
    pub name: String,
    pub age: u32,
}
impl MyTestStruct {
    #[frb(sync)]
    fn new(name: String, age: u32) -> Self {
        Self { name, age }
    }
    pub fn test_mut(&mut self) {
        self.name = "test_mut".to_string();
    }
}

It can't be inited by var myTest = MyTestStruct(); in Dart as referred here.
It can only be initiated like var myTest = MyTestStruct.dcoDecode([18, "Tom"]);, is this designed so on purpose?

2. the field/method issue as I said originally. If the struct is not defined in api folder, then its
   fields and methods are not generated, even with #[frb(opaque)] attribute. As I said before,
   this issue is somehow complex due to the recursive issue---how to find a proper way to generate the type(with its own fields/methods) which is used as the field or the param/return type of methods within the current root struct.

[fzyzcjy]

It can only be initiated like var myTest = MyTestStruct.dcoDecode([18, "Tom"]);, is this designed so on purpose?

No! dcoDecode is not to be used by users. I will rename it later to reflect this.

It must be a bug that it does not understand constructors. Could you please create an issue such that I can have a look?

If the struct is not defined in api folder, then its
fields and methods are not generated

IMHO this is solved in #1954, but I chose to discard it, because I realize that, the meaning of the api folder is the home for things that users want to generate. If a struct is not there, then just like if a function is not there, then we should think users really do not want us to generate anything for that.

Feel free to discuss your scenario if this does not suit your needs!

[dbsxdbsx]

It must be a bug that it does not understand constructors. Could you please create an issue such that I can have a look?

Ok, I've opened a new issue #1981.

IMHO this is solved in #1954, but I chose to discard it, because I realize that, the meaning of the api folder is the home for things that users want to generate. If a struct is not there, then just like if a function is not there, then we should think users really do not want us to generate anything for that.

For example, for a fn API in api/api.rs:

pub async fn update_media_doc_detail(media_doc: MediaDoc) -> MediaDoc {
...
}

Firstly, it is clear that using custom structs as parameters or return types for an API is unavoidable(especially for a big project), as demonstrated by the MediaDoc struct in this example. Secondly, it is a well-known best practice to place structs in a separate module to achieve decoupling, which facilitates future maintenance.

However, your suggestion implies that if methods or fields of any type taken in an API function would be used in Dart, then it must be defined within the same API module. I believe this is a poor practice because it could lead to various types with different semantics being mixed together in the same file or (API ) module, which would complicate the codebase and hinder maintainability.

If you agree on it, then, for the complex parsing part, I would suggest that it is ok to work around to make frb scan and generate those types not defined in API module with a new macro but not by parsing one by one through the API function, which I know is quite complex.

For example, suppose the custom MediaDoc struct is define in src/customiz/mod.rs:

[#frb(generate)]
struct MediaDoc {
}

then, let frb scan ALL the rust folder, once found type with [#frb(generate)], generate it( make sure the type is not already generated by other API fn). Moreover, [#frb(generate)] can be used to tag fields and methods of a type to flexibly control what and what not need to be generated to Dart---This is just a draft design,which may be conflict with current frb, for example, I know there is a [#frb(ignore)].

[fzyzcjy]

(I will check and reply a bit later, hopefully within a day)

[fzyzcjy]

it is a well-known best practice to place structs in a separate module to achieve decoupling, which facilitates future maintenance.

What about something like

src/api/media_doc.rs: `struct MediaDoc { ... }`
src/api/another.rs: `fn update_media_doc_detail() { ... }`

Or, even more folders like

src/api/a/b/c/d.rs: `struct MediaDoc { ... }`
src/api/e/f/g/h/i.rs: `fn update_media_doc_detail() { ... }`

Then it seems that we ensure things are in different modules.

The implementation is simple (as long as the types are in the same crate; it seems no problem if in different modules i.e. files)

[dbsxdbsx]

it is a well-known best practice to place structs in a separate module to achieve decoupling, which facilitates future maintenance.

What about something like

src/api/media_doc.rs: `struct MediaDoc { ... }`
src/api/another.rs: `fn update_media_doc_detail() { ... }`

Or, even more folders like

src/api/a/b/c/d.rs: `struct MediaDoc { ... }`
src/api/e/f/g/h/i.rs: `fn update_media_doc_detail() { ... }`

Then it seems that we ensure things are in different modules.

The implementation is simple (as long as the types are in the same crate; it seems no problem if in different modules i.e. files)

I've though of this approach. This is what I said:

with different semantics being mixed together in the same file or (API ) module.

Here "(API) module" means it may be in API folder(the module) but not in the exact API definition file(like api/api_1.rs).

But still, as I stated, within this way, it implies that almost all custom types should belong to a module called api, is this a good way when programming with frb? I am not sure. Personally, I don't think so, because it makes me somewhat semantically anti-intuitive to arrange some rust types--- This feeling is like, err..... :

This is like saying the human body is an app that needs to integrate various functional organs. The modules used for integration are called APIs. However, in your approach, you would place all the organs and their required functions under the API module. I believe, based on the "single responsibility principle," that different organs should be placed in different modules (rather than in the API module used for bridging functions).

What if asking other core contributors to have a wide and rational discussion on it?

[fzyzcjy]

Here "(API) module" means [...]

Hmm, what if you change the flutter_rust_bridge.yaml to not only scan the api folder? Indeed you can scan the whole crate, or whatever other folder (just remember to mark whatever you do not want to expose as non-public).

What if asking other core contributors to have a wide and rational discussion on it?

Sure, what about creating a new issue firstly (since this thread is already a bit long and contains multiple topics)

[dbsxdbsx]

Sure, what about creating a new issue firstly (since this thread is already a bit long and contains multiple topics)

I've put it in #1987.