Skip to content

Commit

Permalink
Convert the Convert Request/Response types to Proto, refactor Layout …
Browse files Browse the repository at this point in the history
…conversion code (#1965)

With this change we're able to fully remove serde-reflection from the
code base.

This involved a decent amount of refactoring and cleanup of old work to
get things right. My goal with the refactoring was to keep the packages
cleanly separated and to work towards having the ability to build a
DesignCompose app without including anything related to Live Update.
Ultimately I could see the dc_jni crate being split into the layout and
fetch portions, which would compile into their own separate libraries,
and merged in with figma_import and layout.

Some of the messages related to both the doc fetching and layout
processing were a bit scattered and were declared in crates that didn't
actually use them. Before this change the `dc_jni` crate didn't have any
of its own proto messages, it was pulling the ones it needed (Layout
conversion related) from other crates (both `dc_bundle` and `layout`).
The new Convert Request/Response messages would only be used by
`dc_jni`, so it made sense to add a build.rs file to it so that the
generated messages would be native to that crate, rather than being part
of an unrelated crate.

While making that change it made sense to rearrange the proto messages
related to the android interface, so I moved the layout related messages
into a `layout_interface` proto package and put the new Convert messages
into the `live_update` package.

In addition, the layout code had really only been half converted to
proto, we were still parsing them and then converting them into the
original Rust structs. That's all cleaned up now.

We had been manually generating a json string which was decoded into the
ConvertRequest on the rust side, so I converted the ConvertRequest to
Proto and removed the json code. It didn't make sense to use proto on
one side and not the other. It'd probably be cleaner to change the
`jniFetchDoc` method to just send a single proto message instead of the
ConvertRequest, doc_id, version_id and proxy configuration, but that can
be cleaned up another day.

I also decided not to convert the image_session from json to proto
because we don't actually do anything with it on the Kotlin side other
than writing it into the .dcf. I'm simply including the json in the
proto message and removing the redundant storage of the session's bytes
in the docContent class.


Admittedly this PR is bigger than it strictly needed to be. I can split
it into some smaller PRs if requested. I'm sure there's some additional
cleanup that could be done as well, including simplifying the
`jniFetchDoc` function and turning ProxyConfig into a proto struct.
  • Loading branch information
timothyfroehlich authored Jan 13, 2025
1 parent 7315dfa commit 5a70cd0
Show file tree
Hide file tree
Showing 66 changed files with 509 additions and 1,079 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ jobs:
- name: Build all
run: cargo build --all-targets --all-features
- name: Test all
run: cargo test --all-targets --features=reflection,fetch,dcf_info
run: cargo test --all-targets --features=fetch,dcf_info
############ Figma resources
figma-resources:
runs-on: ubuntu-latest
Expand Down
12 changes: 12 additions & 0 deletions .idea/protoeditor.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 5a70cd0

Please sign in to comment.