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

Add json_serializable package check for freezed to/fromJSON #2488

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

satvikpendem
Copy link
Contributor

Changes

Fixes #2468

Checklist

  • An issue to be fixed by this PR is listed above.
  • New tests are added to ensure new features are working. Please refer to this page to see how to add a test.
  • ./frb_internal precommit --mode slow (or fast) is run (it internal runs code generator, does auto formatting, etc).
  • If this PR adds/changes features, documentations (in the ./website folder) are updated.
  • CI is passing. Please refer to this page to see how to solve a failed CI.

Remark for PR creator

  • ./frb_internal --help shows utilities for development.
  • If fzyzcjy does not reply for a few days, maybe he just did not see it, so please ping him.

@satvikpendem satvikpendem mentioned this pull request Jan 8, 2025
5 tasks
@fzyzcjy
Copy link
Owner

fzyzcjy commented Jan 9, 2025

Great job! Btw that PR is not merged (I merged it into branch #2477 to further work on it, and then realize that issue so closed it). Could you please also copy-paste code from there?

@satvikpendem
Copy link
Contributor Author

@fzyzcjy Sure, just did so. I edited it through my browser so I couldn't run the full CI/CD to update the tests however.

@fzyzcjy
Copy link
Owner

fzyzcjy commented Jan 10, 2025

Looks good! However, without running codegen and making CI pass, we cannot know whether it is really done or not... So maybe try to run codegen and pass CI later :)

@satvikpendem
Copy link
Contributor Author

Another issue seems to be that the examples now require the new JSON packages, is there a way to automatically add them or would I have to add them one by one for each example?

@fzyzcjy
Copy link
Owner

fzyzcjy commented Jan 10, 2025

Hmm, seems need to manually edit pubspec.yaml

@satvikpendem
Copy link
Contributor Author

satvikpendem commented Jan 11, 2025

@fzyzcjy Looks like we have an outdated Dart SDK version, should I update it? Not sure how that affects other users though. Or maybe that should be a separate PR you can do since you know more about the Dart SDK versioning for flutter_rust_bridge. I did update the examples just now so they should generate the JSON serde functions.

I am not sure why the formatting is getting messed up. I'm also not sure why sometimes I get errors like [SEVERE] failed to update packages or failed to run build_runner when I run the ./frb_internal precommit_generate command.

@fzyzcjy
Copy link
Owner

fzyzcjy commented Jan 11, 2025

should I update it?

Feel free to do so! (I guess you mean the dart version in CI)

Or maybe that should be a separate PR you can do since you know more about the Dart SDK versioning

No worries, just update it in ci.yaml and post_release.yaml and make it pass. But yes, making it in a separate PR may make it a bit easier to review and track changes.

I am not sure why the formatting is getting messed up. I'm also not sure why sometimes I get errors like [SEVERE] failed to update packages or failed to run build_runner when I run the ./frb_internal precommit_generate command.

Hmm maybe show the exact error logs to see what's going on ...

@satvikpendem
Copy link
Contributor Author

Looks like I didn't install fvm so there haven't been any other errors once I installed that (this detail should be in the docs I think). However, it seems like the Rust compilation is hanging, I just see

    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.57s
     Running `/home/satvik/.cargo/cache/debug/flutter_rust_bridge_codegen generate`
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.58s
     Running `/home/satvik/.cargo/cache/debug/flutter_rust_bridge_codegen generate`
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.58s
     Running `/home/satvik/.cargo/cache/debug/flutter_rust_bridge_codegen generate`
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.60s
     Running `/home/satvik/.cargo/cache/debug/flutter_rust_bridge_codegen generate`
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.62s
     Running `/home/satvik/.cargo/cache/debug/flutter_rust_bridge_codegen generate`
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.64s
     Running `/home/satvik/.cargo/cache/debug/flutter_rust_bridge_codegen generate`
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.66s
     Running `/home/satvik/.cargo/cache/debug/flutter_rust_bridge_codegen generate`
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.65s
     Running `/home/satvik/.cargo/cache/debug/flutter_rust_bridge_codegen generate`
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.67s
     Running `/home/satvik/.cargo/cache/debug/flutter_rust_bridge_codegen generate`
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.68s
     Running `/home/satvik/.cargo/cache/debug/flutter_rust_bridge_codegen generate`
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.69s
     Running `/home/satvik/.cargo/cache/debug/flutter_rust_bridge_codegen generate`

And it just hangs here, the CPU is doing something it looks like but I can't see what it's actually doing.

@satvikpendem
Copy link
Contributor Author

satvikpendem commented Jan 12, 2025

Ah interesting, looks like I have the same formatting issue as #2079, I'll try running the Dart formatter to see if it fixes. Edit, looks like it does for most of them.

I don't see ci.yaml or post_release.yaml anywhere in the repo, where would I find those? Edit, found them, in the .github file. For some reason I didn't see it.

I am not sure what these errors are in the CI to be honest, the error just says ProcessException: Bad exit code

@fzyzcjy
Copy link
Owner

fzyzcjy commented Jan 12, 2025

the CPU is doing something it looks like but I can't see what it's actually doing.

It takes a long time e.g. 15minute on my device. (because it runs frb in debug mode + there are really a ton of generated tests in this repo; maybe one day we should run it in release mode at least to make it faster). Maybe try to wait for e.g. 30minute and see whether it still stucks...

@fzyzcjy
Copy link
Owner

fzyzcjy commented Jan 12, 2025

I am not sure what these errors are in the CI to be honest, the error just says ProcessException: Bad exit code

Could you please paste the full log containing it, then I can have a look

@satvikpendem
Copy link
Contributor Author

Sure, I mean on the GitHub CI, for example: https://github.com/fzyzcjy/flutter_rust_bridge/actions/runs/12731039242/job/35484638988#step:9:1

Errors for other ones are similar

I'll try running for 30 minutes, I already tried 15 and it still hanged.

@fzyzcjy
Copy link
Owner

fzyzcjy commented Jan 13, 2025

Hmm that's weird. To isolate issues, maybe we can firstly make a separate PR that does nothing but bumping the ci.yaml and post_release.yaml's verisons, and in another PR (i.e. this one) that implements json related logic.

@satvikpendem satvikpendem mentioned this pull request Jan 13, 2025
5 tasks
@satvikpendem
Copy link
Contributor Author

#2495 created

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Serialization and deserialization of structs and other fields
2 participants