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

Prevent deregisterElement from deregistering twice in nested object #716

Merged
merged 8 commits into from Jan 4, 2024

Conversation

justiceHui
Copy link
Member

@justiceHui justiceHui commented Jan 4, 2024

What this PR does / why we need it?

  • Prevent the deregisterElement function from deregistering twice in the nested object.
  • Improve time complexity of deregisterElement function $O(2^n)$ to $O(n)$ when $n$ is number of elements to deregistered.

Any background context you want to provide?

Refer to issue #715 for details.

What are the relevant tickets?

Addresses #715

Checklist

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

@CLAassistant
Copy link

CLAassistant commented Jan 4, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

codecov bot commented Jan 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (48c3ccb) 81.76% compared to head (1056e07) 81.77%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #716   +/-   ##
=======================================
  Coverage   81.76%   81.77%           
=======================================
  Files          57       57           
  Lines        4162     4164    +2     
  Branches      805      805           
=======================================
+ Hits         3403     3405    +2     
  Misses        502      502           
  Partials      257      257           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@justiceHui justiceHui changed the title dummy commit Fix deregisterElement must not deregister element twice in nested object Jan 4, 2024
@justiceHui justiceHui marked this pull request as ready for review January 4, 2024 06:20
Copy link
Contributor

@chacha912 chacha912 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.
Looks good!

test/integration/document_test.ts Outdated Show resolved Hide resolved
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.

test/integration/gc_test.ts Show resolved Hide resolved
@hackerwins hackerwins changed the title Fix deregisterElement must not deregister element twice in nested object Prevent deregisterElement from deregistering twice in nested object Jan 4, 2024
@hackerwins hackerwins merged commit 0b3c584 into main Jan 4, 2024
2 checks passed
@hackerwins hackerwins deleted the duplicated_deregister branch January 4, 2024 08:04
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.

None yet

4 participants