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

improve effects example slightly #13

Merged
merged 1 commit into from
May 22, 2024

Conversation

benlesh
Copy link
Contributor

@benlesh benlesh commented May 22, 2024

Just adds a line to release anything that might be held in a closure by the cleanup function.

Not a big deal.

Just adds a line to release anything that might be held in a closure by the cleanup function.
@@ -86,6 +86,7 @@ export function effect(callback) {
return () => {
w.unwatch(computed);
typeof cleanup === "function" && cleanup();
cleanup = undefined;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since cleanup can be any arbitrary user-land function reference, and since the returned function here is being sent out to user-land, there's an opportunity for users to accidentally capture resources they meant to clean up in a closure... albeit very unlikely, this one line is a reasonable defense.

@EisenbergEffect EisenbergEffect merged commit aeeb2bd into proposal-signals:main May 22, 2024
3 checks passed
@github-actions github-actions bot mentioned this pull request May 22, 2024
@NullVoxPopuli NullVoxPopuli added the documentation Improvements or additions to documentation label May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants