Skip to content

chore: remove exposable #9783

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 2 commits into from
Dec 5, 2023
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
47 changes: 31 additions & 16 deletions packages/svelte/src/compiler/phases/3-transform/client/utils.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as b from '../../../utils/builders.js';
import { extract_paths, is_simple_expression } from '../../../utils/ast.js';
import { error } from '../../../errors.js';
import { PROPS_CALL_DEFAULT_VALUE, PROPS_IS_IMMUTABLE } from '../../../../constants.js';

/**
* @template {import('./types').ClientTransformState} State
Expand Down Expand Up @@ -359,29 +360,43 @@ export function get_props_method(binding, state, name, default_value) {
(state.analysis.immutable ? binding.reassigned : binding.mutated);

if (needs_source) {
args.push(b.literal(state.analysis.immutable));
}
let flags = 0;

if (default_value) {
// To avoid eagerly evaluating the right-hand-side, we wrap it in a thunk if necessary
if (is_simple_expression(default_value)) {
args.push(default_value);
} else {
if (
default_value.type === 'CallExpression' &&
default_value.callee.type === 'Identifier' &&
default_value.arguments.length === 0
) {
args.push(default_value.callee);
/** @type {import('estree').Expression | undefined} */
let arg;

if (state.analysis.immutable) {
flags |= PROPS_IS_IMMUTABLE;
}

if (default_value) {
// To avoid eagerly evaluating the right-hand-side, we wrap it in a thunk if necessary
if (is_simple_expression(default_value)) {
arg = default_value;
} else {
args.push(b.thunk(default_value));
if (
default_value.type === 'CallExpression' &&
default_value.callee.type === 'Identifier' &&
default_value.arguments.length === 0
) {
arg = default_value.callee;
} else {
arg = b.thunk(default_value);
}

flags |= PROPS_CALL_DEFAULT_VALUE;
}
}

args.push(b.true);
if (flags || arg) {
args.push(b.literal(flags));
if (arg) args.push(arg);
}

return b.call('$.prop_source', ...args);
}

return b.call(needs_source ? '$.prop_source' : '$.prop', ...args);
return b.call('$.prop', ...args);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -795,32 +795,17 @@ function serialize_inline_component(node, component_name, context) {
push_prop(
b.get(attribute.name, [
b.return(
b.call(
'$.exposable',
b.thunk(
/** @type {import('estree').Expression} */ (context.visit(attribute.expression))
)
)
/** @type {import('estree').Expression} */ (context.visit(attribute.expression))
)
])
);
// If the binding is just a reference to a top level state variable
// we don't need a setter as the inner component can write to the signal directly
const binding =
attribute.expression.type !== 'Identifier'
? null
: context.state.scope.get(attribute.expression.name);
if (
binding === null ||
(binding.kind !== 'state' && binding.kind !== 'prop' && binding.kind !== 'rest_prop')
) {
const assignment = b.assignment('=', attribute.expression, b.id('$$value'));
push_prop(
b.set(attribute.name, [
b.stmt(serialize_set_binding(assignment, context, () => context.visit(assignment)))
])
);
}

const assignment = b.assignment('=', attribute.expression, b.id('$$value'));
push_prop(
b.set(attribute.name, [
b.stmt(serialize_set_binding(assignment, context, () => context.visit(assignment)))
])
);
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions packages/svelte/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ export const EACH_IS_CONTROLLED = 1 << 3;
export const EACH_IS_ANIMATED = 1 << 4;
export const EACH_IS_IMMUTABLE = 1 << 6;

export const PROPS_IS_IMMUTABLE = 1;
export const PROPS_CALL_DEFAULT_VALUE = 1 << 1;

/** List of Element events that will be delegated */
export const DelegatedEvents = [
'beforeinput',
Expand Down
6 changes: 1 addition & 5 deletions packages/svelte/src/internal/client/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import {
untrack,
effect,
flushSync,
expose,
safe_not_equal,
current_block,
source,
Expand Down Expand Up @@ -1193,10 +1192,7 @@ export function bind_prop(props, prop, value) {
/** @param {V | null} value */
const update = (value) => {
const current_props = unwrap(props);
const signal = expose(() => current_props[prop]);
if (is_signal(signal)) {
set(signal, value);
} else if (Object.getOwnPropertyDescriptor(current_props, prop)?.set !== undefined) {
if (get_descriptor(current_props, prop)?.set !== undefined) {
current_props[prop] = value;
}
};
Expand Down
92 changes: 13 additions & 79 deletions packages/svelte/src/internal/client/runtime.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { DEV } from 'esm-env';
import { subscribe_to_store } from '../../store/utils.js';
import { EMPTY_FUNC, run_all } from '../common.js';
import { get_descriptors, is_array } from './utils.js';
import { get_descriptor, get_descriptors, is_array } from './utils.js';
import { PROPS_CALL_DEFAULT_VALUE, PROPS_IS_IMMUTABLE } from '../../constants.js';

export const SOURCE = 1;
export const DERIVED = 1 << 1;
Expand Down Expand Up @@ -30,8 +31,7 @@ let current_scheduler_mode = FLUSH_MICROTASK;
// Used for handling scheduling
let is_micro_task_queued = false;
let is_task_queued = false;
// Used for exposing signals
let is_signal_exposed = false;

// Handle effect queues

/** @type {import('./types.js').EffectSignal[]} */
Expand Down Expand Up @@ -63,8 +63,6 @@ export let current_untracking = false;
/** Exists to opt out of the mutation validation for stores which may be set for the first time during a derivation */
let ignore_mutation_validation = false;

/** @type {null | import('./types.js').Signal} */
let current_captured_signal = null;
// If we are working with a get() chain that has no active container,
// to prevent memory leaks, we skip adding the consumer.
let current_skip_consumer = false;
Expand Down Expand Up @@ -800,23 +798,6 @@ export function unsubscribe_on_destroy(stores) {
});
}

/**
* Wraps a function and marks execution context so that the last signal read from can be captured
* using the `expose` function.
* @template V
* @param {() => V} fn
* @returns {V}
*/
export function exposable(fn) {
const previous_is_signal_exposed = is_signal_exposed;
try {
is_signal_exposed = true;
return fn();
} finally {
is_signal_exposed = previous_is_signal_exposed;
}
}

/**
* @template V
* @param {import('./types.js').Signal<V>} signal
Expand All @@ -836,10 +817,6 @@ export function get(signal) {
return signal.v;
}

if (is_signal_exposed && current_should_capture_signal) {
current_captured_signal = signal;
}

if (is_signals_recorded) {
captured_signals.add(signal);
}
Expand Down Expand Up @@ -906,31 +883,6 @@ export function set_sync(signal, value) {
flushSync(() => set(signal, value));
}

/**
* Invokes a function and captures the last signal that is read during the invocation
* if that signal is read within the `exposable` function context.
* If a signal is captured, it returns the signal instead of the read value.
* @template V
* @param {() => V} possible_signal_fn
* @returns {any}
*/
export function expose(possible_signal_fn) {
const previous_captured_signal = current_captured_signal;
const previous_should_capture_signal = current_should_capture_signal;
current_captured_signal = null;
current_should_capture_signal = true;
try {
const value = possible_signal_fn();
if (current_captured_signal === null) {
return value;
}
return current_captured_signal;
} finally {
current_captured_signal = previous_captured_signal;
current_should_capture_signal = previous_should_capture_signal;
}
}

/**
* Invokes a function and captures all signals that are read during the invocation,
* then invalidates them.
Expand Down Expand Up @@ -1463,35 +1415,19 @@ export function is_store(val) {
* @template V
* @param {import('./types.js').MaybeSignal<Record<string, unknown>>} props_obj
* @param {string} key
* @param {boolean} immutable
* @param {number} flags
* @param {V | (() => V)} [default_value]
* @param {boolean} [call_default_value]
* @returns {import('./types.js').Signal<V> | (() => V)}
*/
export function prop_source(props_obj, key, immutable, default_value, call_default_value) {
export function prop_source(props_obj, key, flags, default_value) {
const call_default_value = (flags & PROPS_CALL_DEFAULT_VALUE) !== 0;
const immutable = (flags & PROPS_IS_IMMUTABLE) !== 0;

const props = is_signal(props_obj) ? get(props_obj) : props_obj;
const possible_signal = /** @type {import('./types.js').MaybeSignal<V>} */ (
expose(() => props[key])
);
const update_bound_prop = Object.getOwnPropertyDescriptor(props, key)?.set;
const update_bound_prop = get_descriptor(props, key)?.set;
let value = props[key];
const should_set_default_value = value === undefined && default_value !== undefined;

if (
is_signal(possible_signal) &&
possible_signal.v === value &&
update_bound_prop === undefined
) {
if (should_set_default_value) {
set(
possible_signal,
// @ts-expect-error would need a cumbersome method overload to type this
call_default_value ? default_value() : default_value
);
}
return possible_signal;
}

if (should_set_default_value) {
value =
// @ts-expect-error would need a cumbersome method overload to type this
Expand Down Expand Up @@ -1534,7 +1470,7 @@ export function prop_source(props_obj, key, immutable, default_value, call_defau
}
});

if (is_signal(possible_signal) && update_bound_prop !== undefined) {
if (update_bound_prop !== undefined) {
let ignore_first = !should_set_default_value;
sync_effect(() => {
// Before if to ensure signal dependency is registered
Expand All @@ -1548,11 +1484,9 @@ export function prop_source(props_obj, key, immutable, default_value, call_defau
return;
}

if (not_equal(immutable, propagating_value, possible_signal.v)) {
Copy link
Member

Choose a reason for hiding this comment

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

Why can we remove this? I'm a bit surprised that no tests fail because removing this seems unrelated. Is it because the parent now always decides what to do with the update?

ignore_next1 = true;
did_update_to_defined = true;
untrack(() => update_bound_prop(propagating_value));
}
ignore_next1 = true;
did_update_to_defined = true;
untrack(() => update_bound_prop(propagating_value));
});
}

Expand Down
2 changes: 0 additions & 2 deletions packages/svelte/src/internal/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ export {
set,
set_sync,
invalidate_inner_signals,
expose,
exposable,
source,
mutable_source,
derived,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as $ from "svelte/internal";
export default function Svelte_element($$anchor, $$props) {
$.push($$props, true);

let tag = $.prop_source($$props, "tag", true, 'hr');
let tag = $.prop_source($$props, "tag", 1, 'hr');
/* Init */
var fragment = $.comment($$anchor);
var node = $.child_frag(fragment);
Expand Down