-
Notifications
You must be signed in to change notification settings - Fork 99
refactor: remove version field & remove recursed flag #95
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the reactive system by removing version-based link tracking and the Recursed flag, simplifying the change propagation logic.
- Removes the
versionfield fromLinkinterface andcycleglobal variable used for version tracking - Removes the
ReactiveFlags.Recursedflag (value 8) from the reactive flags enum - Simplifies the propagation logic by removing the
isValidLink()function and associated conditional branches
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/system.ts | Removes version field from Link interface, removes Recursed flag enum value, updates link() function signature to remove version parameter, simplifies propagation logic by removing isValidLink() function and version/Recursed checks |
| src/index.ts | Removes cycle global variable, updates all link() function calls to remove version/cycle parameter from effect(), effectScope(), computedOper(), and signalOper() functions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
bench before remove: bench after remove: |
After reviewing the source code, I noticed that some parts of the logic do not appear to have any observable effect:
Recursedflag seems to be unnecessary.cyclefield (Link.version) also appears to be redundant.isValidLinkcheck insidepropagatedoes not seem to be required.After removing these parts, all existing tests still pass successfully, and the overall performance appears to improve.
However, I am not fully certain whether removing these pieces is entirely safe in all scenarios. I would appreciate guidance from the maintainers or reviewers on whether this simplification is acceptable.
This PR proposes removing the above logic to simplify the implementation and improve performance.