-
-
Notifications
You must be signed in to change notification settings - Fork 111
Add type as Node | undefined to head #674
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
Conversation
|
✅ Deploy Preview for preact-signals-demo ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Your PR needs a changeset |
Co-authored-by: Jovi De Croock <[email protected]>
I added a changeset. I hope I did it right. I'm really sorry if I messed it up. I have never contributed to open source before. |
Co-authored-by: Ryan Christian <[email protected]>
Sorry for the delay here, been quite busy. It looks like this change isn't quite as innocent as it appears, judging by the state of the CI. We might be able to get away with removing the first check instead, though that might hurt benchmarks a bit. Would need to take a deeper look into that. |
The check is not redundant. The first This is what the comment "If there's a new version of the dependency before or after refreshing" above the code alludes to. |
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.
^What @jviide said. It's a performance optimization.
Tried to address the ambiguousness of the code in #690. Thank you @nishan-singh for pointing this out 👍 |
I removed duplicated node source version check in
needToRecompute
func and make the head's typeNode | undefined
incleanUpSources
func.