Skip to content

Improve communication of serialised Realm objects between device and plugin #123

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.

Sign up for GitHub

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

Jump to bottom
Merged
gagik merged 21 commits into from
Jan 18, 2023
Merged

Improve communication of serialised Realm objects between device and plugin #123

gagik merged 21 commits into from
Jan 18, 2023

Conversation

@gagik
Copy link
Contributor

@gagik gagik commented Jan 12, 2023 •
edited
Loading

This PR...

TODO:

  • Empty collections being sent over not handled correctly
  • Embedded object modification not handled correctly
  • Test modification of different types
  • Test expanding data

@cla-bot cla-bot bot added the cla: yes label Jan 12, 2023
@gagik gagik changed the base branch from main to gagik/typescriptrefactoring January 12, 2023 13:05
@gagik gagik mentioned this pull request Jan 12, 2023
Closed
This was linked to issues Jan 12, 2023
Flattening of linked objects #85
Closed
Special property names #86
Closed
Object Key in data inspector #93
Closed
Circular references #96
Closed
Embedded Object Support #113
Closed
@gagik gagik force-pushed the gagik/typescriptrefactoring branch from 3f036e3 to fdd81eb Compare January 12, 2023 14:29
@gagik gagik mentioned this pull request Jan 12, 2023
Merged
@gagik gagik force-pushed the gagik/betterserialization branch from 2d249d7 to fe0e949 Compare January 13, 2023 16:00
@gagik gagik marked this pull request as ready for review January 16, 2023 16:50
@gagik gagik force-pushed the gagik/betterserialization branch from cae0d65 to 536e421 Compare January 17, 2023 10:26
@gagik gagik force-pushed the gagik/betterserialization branch from 536e421 to 03d88d3 Compare January 17, 2023 11:53
takameyer
takameyer approved these changes Jan 17, 2023
View reviewed changes
Copy link
Contributor

@takameyer takameyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really happy about all the work you've put into this. Looks like the flipper plugin is now useable!
My only suggestion is to consider breaking up "common" and "shared" and getting some of those types closer to the component using them. But this could be done later, just wanted to provide something to think about.
LGTM 🚢

flipper-plugin-realm/src/components/objectManipulation/types/ObjectInput.tsx
if(isEmbedded) {
content = ``;
}
// TODO: Fetch object with getObject to display primary key instead of objectKey
Copy link
Contributor

@takameyer takameyer Jan 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you want to address this in a separate PR, or is this doable now?

Copy link
Contributor Author

@gagik gagik Jan 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of separate PR. although I just thought of a way to make this simpler, i.e. just send over primaryKey if it exists along with objectType / objectKey for references, so I might just do this. I generally avoid leaving TODOs like this but because of the state of the code right now this seemed like the quickest way to leave some bits for future planning.

Base automatically changed from gagik/typescriptrefactoring to main January 18, 2023 12:28
@gagik gagik merged commit 7da4dec into main Jan 18, 2023
@gagik gagik linked an issue Jan 18, 2023 that may be closed by this pull request
Implement a new approach to communicating serialised Realm objects between device and plugin #121
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@takameyer takameyer takameyer approved these changes

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

3 participants

@gagik @takameyer