Skip to content

chore: optimise effects that only exist to return a teardown #11936

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

Merged
merged 6 commits into from
Jun 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/lemon-meals-appear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

chore: optimise effects that only exist to return a teardown
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { DEV } from 'esm-env';
import { render_effect, effect } from '../../../reactivity/effects.js';
import { render_effect, effect, teardown } from '../../../reactivity/effects.js';
import { stringify } from '../../../render.js';
import { listen_to_event_and_reset_event } from './shared.js';
import * as e from '../../../errors.js';
Expand Down Expand Up @@ -103,14 +103,12 @@ export function bind_group(inputs, group_index, input, get_value, update) {
}
});

render_effect(() => {
return () => {
var index = binding_group.indexOf(input);
teardown(() => {
var index = binding_group.indexOf(input);

if (index !== -1) {
binding_group.splice(index, 1);
}
};
if (index !== -1) {
binding_group.splice(index, 1);
}
});

effect(() => {
Expand Down Expand Up @@ -189,6 +187,7 @@ export function bind_files(input, get_value, update) {
listen_to_event_and_reset_event(input, 'change', () => {
update(input.files);
});

render_effect(() => {
input.files = get_value();
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { hydrating } from '../../hydration.js';
import { render_effect, effect } from '../../../reactivity/effects.js';
import { render_effect, effect, teardown } from '../../../reactivity/effects.js';
import { listen } from './shared.js';

/** @param {TimeRanges} ranges */
Expand Down Expand Up @@ -52,7 +52,7 @@ export function bind_current_time(media, get_value, update) {
updating = false;
});

render_effect(() => () => cancelAnimationFrame(raf_id));
teardown(() => cancelAnimationFrame(raf_id));
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { render_effect } from '../../../reactivity/effects.js';
import { teardown } from '../../../reactivity/effects.js';
import { get_descriptor } from '../../../utils.js';

/**
Expand All @@ -15,7 +15,7 @@ export function bind_prop(props, prop, value) {

if (desc && desc.set) {
props[prop] = value;
render_effect(() => () => {
teardown(() => {
props[prop] = null;
});
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { render_effect } from '../../../reactivity/effects.js';
import { teardown } from '../../../reactivity/effects.js';
import { add_form_reset_listener } from '../misc.js';

/**
Expand All @@ -18,12 +18,10 @@ export function listen(target, events, handler, call_handler_immediately = true)
target.addEventListener(name, handler);
}

render_effect(() => {
return () => {
for (var name of events) {
target.removeEventListener(name, handler);
}
};
teardown(() => {
for (var name of events) {
target.removeEventListener(name, handler);
}
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { effect, render_effect } from '../../../reactivity/effects.js';
import { untrack } from '../../../runtime.js';
import { effect, teardown } from '../../../reactivity/effects.js';

/**
* Resize observer singleton.
Expand Down Expand Up @@ -89,7 +88,7 @@ export function bind_resize_observer(element, type, update) {
: resize_observer_device_pixel_content_box;

var unsub = observer.observe(element, /** @param {any} entry */ (entry) => update(entry[type]));
render_effect(() => unsub);
teardown(unsub);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { render_effect } from '../../../reactivity/effects.js';
import { render_effect, teardown } from '../../../reactivity/effects.js';
import { listen } from './shared.js';

/**
Expand Down Expand Up @@ -57,10 +57,8 @@ export function bind_property(property, event_name, element, set, get) {

// @ts-ignore
if (element === document.body || element === window || element === document) {
render_effect(() => {
return () => {
element.removeEventListener(event_name, handler);
};
teardown(() => {
element.removeEventListener(event_name, handler);
});
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { effect, render_effect } from '../../../reactivity/effects.js';
import { effect, render_effect, teardown } from '../../../reactivity/effects.js';
import { listen } from './shared.js';

/**
Expand Down Expand Up @@ -51,10 +51,8 @@ export function bind_window_scroll(type, get_value, update) {
// Browsers don't fire the scroll event for the initial scroll position when scroll style isn't set to smooth
effect(target_handler);

render_effect(() => {
return () => {
removeEventListener('scroll', target_handler);
};
teardown(() => {
removeEventListener('scroll', target_handler);
});
}

Expand Down
8 changes: 3 additions & 5 deletions packages/svelte/src/internal/client/dom/elements/events.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { render_effect } from '../../reactivity/effects.js';
import { render_effect, teardown } from '../../reactivity/effects.js';
import { all_registered_events, root_event_handles } from '../../render.js';
import { define_property, is_array } from '../../utils.js';
import { hydrating } from '../hydration.js';
Expand Down Expand Up @@ -98,10 +98,8 @@ export function event(event_name, dom, handler, capture, passive) {

// @ts-ignore
if (dom === document.body || dom === window || dom === document) {
render_effect(() => {
return () => {
dom.removeEventListener(event_name, target_handler, options);
};
teardown(() => {
dom.removeEventListener(event_name, target_handler, options);
});
}
}
Expand Down
18 changes: 14 additions & 4 deletions packages/svelte/src/internal/client/reactivity/effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ import {
ROOT_EFFECT,
EFFECT_TRANSPARENT,
DERIVED,
UNOWNED
UNOWNED,
CLEAN
} from '../constants.js';
import { set } from './sources.js';
import { remove } from '../dom/reconciler.js';
Expand Down Expand Up @@ -68,7 +69,7 @@ export function push_effect(effect, parent_effect) {

/**
* @param {number} type
* @param {(() => void | (() => void))} fn
* @param {null | (() => void | (() => void))} fn
* @param {boolean} sync
* @returns {import('#client').Effect}
*/
Expand Down Expand Up @@ -121,7 +122,7 @@ function create_effect(type, fn, sync) {
} finally {
set_is_flushing_effect(previously_flushing_effect);
}
} else {
} else if (fn !== null) {
schedule_effect(effect);
}

Expand All @@ -144,6 +145,16 @@ export function effect_active() {
return false;
}

/**
* @param {() => void} fn
*/
export function teardown(fn) {
const effect = create_effect(RENDER_EFFECT, null, false);
set_signal_status(effect, CLEAN);
effect.teardown = fn;
return effect;
}

/**
* Internal representation of `$effect(...)`
* @param {() => void | (() => void)} fn
Expand Down Expand Up @@ -364,7 +375,6 @@ export function destroy_effect(effect, remove_dom = true) {
effect.dom =
effect.deps =
effect.parent =
// @ts-expect-error
effect.fn =
null;
}
Expand Down
4 changes: 2 additions & 2 deletions packages/svelte/src/internal/client/reactivity/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export interface Value<V = unknown> extends Signal {

export interface Reaction extends Signal {
/** The reaction function */
fn: Function;
fn: null | Function;
/** Signals that this signal reads from */
deps: null | Value[];
/** First child effect created inside this signal */
Expand All @@ -40,7 +40,7 @@ export interface Effect extends Reaction {
/** The associated component context */
ctx: null | ComponentContext;
/** The effect function */
fn: () => void | (() => void);
fn: null | (() => void | (() => void));
/** The teardown function returned from the effect function */
teardown: null | (() => void);
/** Transition managers created with `$.transition` */
Expand Down
4 changes: 2 additions & 2 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ function handle_error(error, effect, component_context) {

const component_stack = [];

const effect_name = effect.fn.name;
const effect_name = effect.fn?.name;

if (effect_name) {
component_stack.push(effect_name);
Expand Down Expand Up @@ -358,7 +358,7 @@ export function execute_reaction_fn(signal) {
current_untracking = false;

try {
let res = (0, signal.fn)();
let res = /** @type {Function} */ (0, signal.fn)();
let dependencies = /** @type {import('#client').Value<unknown>[]} **/ (signal.deps);
if (current_dependencies !== null) {
let i;
Expand Down
Loading