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 the toJS method to the ObjectProxy's handler #449

Merged
merged 2 commits into from
Jan 31, 2023
Merged

Add the toJS method to the ObjectProxy's handler #449

merged 2 commits into from
Jan 31, 2023

Conversation

chacha912
Copy link
Contributor

What this PR does / why we need it?

Add the toJS method to the ObjectProxy's handler.
Previously, toJSON returns a JSON string.
It requires an extra step of JSON.parse to use JSON object.
After this PR, it is easier to use the JSON object with toJS method.

doc.subscribe((event) => {
  // as-is
  const rootObject = JSON.parse(doc.getRoot().toJSON());

  // to-be
  const rootObject = doc.getRoot().toJS();
});

Any background context you want to provide?

What are the relevant tickets?

Fixes #444

Checklist

  • Added relevant tests or not required
  • Didn't break anything

@codecov
Copy link

codecov bot commented Jan 30, 2023

Codecov Report

Merging #449 (14ba735) into main (9e1e776) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #449      +/-   ##
==========================================
+ Coverage   89.27%   89.30%   +0.02%     
==========================================
  Files          69       69              
  Lines        5296     5309      +13     
  Branches      530      531       +1     
==========================================
+ Hits         4728     4741      +13     
  Misses        385      385              
  Partials      183      183              
Impacted Files Coverage Δ
src/document/document.ts 79.00% <ø> (ø)
src/document/json/object.ts 88.23% <100.00%> (+0.54%) ⬆️
test/integration/object_test.ts 100.00% <100.00%> (ø)
src/yorkie.ts 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@hackerwins hackerwins self-requested a review January 30, 2023 10:06
Copy link
Member

@hackerwins hackerwins left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution.

What about adding test codes for this?

@chacha912 chacha912 requested a review from hackerwins January 31, 2023 03:34
@chacha912 chacha912 marked this pull request as draft January 31, 2023 07:03
@chacha912 chacha912 marked this pull request as ready for review January 31, 2023 07:39
Copy link
Member

@hackerwins hackerwins left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution.

@hackerwins hackerwins merged commit 98d42f9 into yorkie-team:main Jan 31, 2023
hunkim98 pushed a commit to hunkim98/yorkie-js-sdk that referenced this pull request Jul 12, 2023
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.

I need to use toJS() function in CRDTObject Proxy
2 participants