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

Remove use entities #786

Merged
merged 13 commits into from
Feb 12, 2025
Merged

Remove use entities #786

merged 13 commits into from
Feb 12, 2025

Conversation

kmcginnes
Copy link
Collaborator

@kmcginnes kmcginnes commented Feb 11, 2025

Description

The hook useEntities() was used for nearly everything in the past. I have slowly been moving logic out of that hook, and the corresponding Recoil selector entitiesSelector, for the last 2 months. This is the final removal of the old code.

  • Moved all entity adding logic to useAddToGraph()
    • Make sure schema is updated with any new types or attributes
    • Update import to use add to graph hook
  • Moved all entity removal logic to useRemoveFromGraph()
    • Clean up any tangential state (selected entities, focused entities, etc)
  • Remove useEntities()
    • Move all tests to new hooks
    • Remove entitiesSelector, nodesSelector, and edgesSelector
    • Fix filteredEdgesSelector to fix issue where node doesn't exist for an edge
  • Use ConfigurationId as type for active connection

Validation

  • Tested in all three query engines
  • Tested adding and removing many entities
  • Tested filtering, selection, and focus

Related Issues

Check List

  • I confirm that my contribution is made under the terms of the Apache 2.0
    license.
  • I have run pnpm checks to ensure code compiles and meets standards.
  • I have run pnpm test to check if all tests are passing.
  • I have covered new added functionality with unit tests if necessary.
  • I have added an entry in the Changelog.md.

@kmcginnes kmcginnes marked this pull request as ready for review February 11, 2025 20:38
@andreachild
Copy link

LGTM

@kmcginnes kmcginnes merged commit 33bd7a0 into aws:main Feb 12, 2025
2 checks passed
@kmcginnes kmcginnes deleted the remove-use-entities branch February 12, 2025 23:40
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.

App crash during import when node doesn't exist for edge
2 participants