From f076bbc7d4f73a2a380b72be723ae86b58abe06d Mon Sep 17 00:00:00 2001 From: godzylinux Date: Tue, 7 May 2024 20:16:48 +0330 Subject: [PATCH 01/71] a new take on reactivity package --- packages/svelte/src/reactivity/set.js | 173 ++---------------------- packages/svelte/src/reactivity/utils.js | 84 ++++++++++++ 2 files changed, 98 insertions(+), 159 deletions(-) diff --git a/packages/svelte/src/reactivity/set.js b/packages/svelte/src/reactivity/set.js index 809109846a92..453055a9ba0f 100644 --- a/packages/svelte/src/reactivity/set.js +++ b/packages/svelte/src/reactivity/set.js @@ -1,161 +1,16 @@ -import { DEV } from 'esm-env'; -import { source, set } from '../internal/client/reactivity/sources.js'; -import { get } from '../internal/client/runtime.js'; - -var read_methods = ['forEach', 'isDisjointFrom', 'isSubsetOf', 'isSupersetOf']; -var set_like_methods = ['difference', 'intersection', 'symmetricDifference', 'union']; - -var inited = false; - -/** - * @template T - * @extends {Set} - */ -export class ReactiveSet extends Set { - /** @type {Map>} */ - #sources = new Map(); - #version = source(0); - #size = source(0); - - /** - * @param {Iterable | null | undefined} [value] - */ - constructor(value) { - super(); - - // If the value is invalid then the native exception will fire here - if (DEV) new Set(value); - - if (value) { - for (var element of value) { - super.add(element); - } - this.#size.v = super.size; - } - - if (!inited) this.#init(); - } - - // We init as part of the first instance so that we can treeshake this class - #init() { - inited = true; - - var proto = ReactiveSet.prototype; - var set_proto = Set.prototype; - - for (const method of read_methods) { - // @ts-ignore - proto[method] = function (...v) { - get(this.#version); - // @ts-ignore - return set_proto[method].apply(this, v); - }; +import { make_reactive } from './utils.js'; + +export const ReactiveSet = make_reactive(Set, { + mutation_properties: /** @type {const} */ (['add', 'clear', 'delete']), + interceptors: { + add: (value, property, ...params) => { + return !value.has(params[0]); + }, + clear: (value, property, ...params) => { + return value.size !== 0; + }, + delete: (value, property, ...params) => { + return !value.has(params[0]); } - - for (const method of set_like_methods) { - // @ts-ignore - proto[method] = function (...v) { - get(this.#version); - // @ts-ignore - var set = /** @type {Set} */ (set_proto[method].apply(this, v)); - return new ReactiveSet(set); - }; - } - } - - #increment_version() { - set(this.#version, this.#version.v + 1); - } - - /** @param {T} value */ - has(value) { - var sources = this.#sources; - var s = sources.get(value); - - if (s === undefined) { - var ret = super.has(value); - if (ret) { - s = source(true); - sources.set(value, s); - } else { - // We should always track the version in case - // the Set ever gets this value in the future. - get(this.#version); - return false; - } - } - - get(s); - return super.has(value); - } - - /** @param {T} value */ - add(value) { - var sources = this.#sources; - var res = super.add(value); - var s = sources.get(value); - - if (s === undefined) { - sources.set(value, source(true)); - set(this.#size, super.size); - this.#increment_version(); - } else { - set(s, true); - } - - return res; - } - - /** @param {T} value */ - delete(value) { - var sources = this.#sources; - var s = sources.get(value); - var res = super.delete(value); - - if (s !== undefined) { - sources.delete(value); - set(this.#size, super.size); - set(s, false); - this.#increment_version(); - } - - return res; - } - - clear() { - var sources = this.#sources; - - if (super.size !== 0) { - set(this.#size, 0); - for (var s of sources.values()) { - set(s, false); - } - this.#increment_version(); - sources.clear(); - } - super.clear(); - } - - keys() { - get(this.#version); - return super.keys(); - } - - values() { - get(this.#version); - return super.values(); - } - - entries() { - get(this.#version); - return super.entries(); - } - - [Symbol.iterator]() { - return this.keys(); - } - - get size() { - return get(this.#size); } -} +}); diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index cc71bf094c19..d7b02e38c163 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -1,3 +1,6 @@ +import { source, set } from '../internal/client/reactivity/sources.js'; +import { get } from '../internal/client/runtime.js'; + /** * @template T * @template U @@ -27,3 +30,84 @@ export function map(iterable, fn, name) { function get_this() { return this; } + +/** + * @template TEntityInstance + * @template {(keyof TEntityInstance)[]} TMutationProperties + * @typedef {Recordboolean>} Interceptors - return false if you want to prevent reactivity for this call/get + */ + +/** + * @template TEntityInstance + * @template {(keyof TEntityInstance)[]} TMutationProperties + * @typedef {object} Options + * @prop {TMutationProperties} mutation_properties - an array of property names on `TEntityInstance`. when calling a property on `TEntityInstance`, if the property name exists in this array, then mentioned property is made reactive. + * @prop {Interceptors} [interceptors={}] - if the property names in `mutation_properties` shouldn't be always reactive, such calling `set.add(2)` twice, you can prevent the reactivity by returning false from these interceptors + */ + +/** + * @template {new (...args: any) => any} TEntity - the entity we want to make reactive + * @template {(keyof InstanceType)[]} TMutationProperties + * @param {TEntity} Entity - the class/function we want to make reactive + * @param {Options, TMutationProperties>} options - configurations for how reactivity works for this entity + * @returns {TEntity} + */ +export const make_reactive = (Entity, options) => { + /** + * @template {InstanceType} TEntityInstance + * @template {keyof TEntityInstance} TProperty + * @param {import('#client').Source} target + * @param {TProperty} property + * @param {TEntityInstance} entity_instance + * @param {TEntityInstance[TProperty] extends (...args: any)=>any ? Parameters: never} params + */ + function notify_if_required(target, property, entity_instance, ...params) { + if (options.interceptors?.[property]?.(entity_instance, property, ...params) === false) { + // if interceptor said to not make this call reactive then bailout + return; + } + + if (options.mutation_properties.some((v) => v === property)) { + set(target, target.v + 1); + } else { + get(target); + } + } + + /** + * @template {ConstructorParameters} TParams + */ + // we return a class so that the caller can call it with new + // @ts-ignore + return class { + /** + * @param {TParams} params + * @returns + */ + constructor(...params) { + const sig = source(0); + + return new Proxy(new Entity(...params), { + get(target, property) { + const orig_property = target[property]; + + let result; + + if (typeof orig_property === 'function') { + // Bind functions directly to the Set + result = ((/** @type {any} */ ...params) => { + notify_if_required(sig, property, target, ...params); + return orig_property.bind(target)(...params); + }).bind(target); + } else { + // Properly handle getters + result = Reflect.get(target, property, target); + notify_if_required(sig, property, target); + } + + return result; + } + }); + } + }; +}; From 72cb168cb480f2b40e79cf4be4556c8e40656dd9 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Tue, 7 May 2024 20:48:05 +0330 Subject: [PATCH 02/71] removed unnecessary as const cast --- packages/svelte/src/reactivity/set.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/reactivity/set.js b/packages/svelte/src/reactivity/set.js index 453055a9ba0f..a618d9fd4039 100644 --- a/packages/svelte/src/reactivity/set.js +++ b/packages/svelte/src/reactivity/set.js @@ -1,7 +1,7 @@ import { make_reactive } from './utils.js'; export const ReactiveSet = make_reactive(Set, { - mutation_properties: /** @type {const} */ (['add', 'clear', 'delete']), + mutation_properties: ['add', 'clear', 'delete'], interceptors: { add: (value, property, ...params) => { return !value.has(params[0]); From 54e33aa10ddc6c1ace49d11b0be5d208b0e2d26e Mon Sep 17 00:00:00 2001 From: godzylinux Date: Tue, 7 May 2024 20:51:45 +0330 Subject: [PATCH 03/71] fixed condition of `delete` in ReactiveSet --- packages/svelte/src/reactivity/set.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/reactivity/set.js b/packages/svelte/src/reactivity/set.js index a618d9fd4039..a98302e3e3f4 100644 --- a/packages/svelte/src/reactivity/set.js +++ b/packages/svelte/src/reactivity/set.js @@ -10,7 +10,7 @@ export const ReactiveSet = make_reactive(Set, { return value.size !== 0; }, delete: (value, property, ...params) => { - return !value.has(params[0]); + return value.has(params[0]); } } }); From 10c8197478284c9109a93fd2380b2618c4e2acee Mon Sep 17 00:00:00 2001 From: godzylinux Date: Tue, 7 May 2024 21:10:14 +0330 Subject: [PATCH 04/71] better describe what `mutation_properties` does --- packages/svelte/src/reactivity/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index d7b02e38c163..5484f509e226 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -41,7 +41,7 @@ function get_this() { * @template TEntityInstance * @template {(keyof TEntityInstance)[]} TMutationProperties * @typedef {object} Options - * @prop {TMutationProperties} mutation_properties - an array of property names on `TEntityInstance`. when calling a property on `TEntityInstance`, if the property name exists in this array, then mentioned property is made reactive. + * @prop {TMutationProperties} mutation_properties - an array of property names on `TEntityInstance`. when calling a property on `TEntityInstance`, if the property name exists in this array, then mentioned property causes reactivity. * @prop {Interceptors} [interceptors={}] - if the property names in `mutation_properties` shouldn't be always reactive, such calling `set.add(2)` twice, you can prevent the reactivity by returning false from these interceptors */ From c163ec75ccd54fc580e0f5798dd5a6cb394f1299 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 8 May 2024 12:17:42 +0330 Subject: [PATCH 05/71] improved types --- packages/svelte/src/reactivity/utils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 5484f509e226..ad4c5a019fce 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -34,7 +34,7 @@ function get_this() { /** * @template TEntityInstance * @template {(keyof TEntityInstance)[]} TMutationProperties - * @typedef {Recordboolean>} Interceptors - return false if you want to prevent reactivity for this call/get + * @typedef {Recordboolean>} Interceptors - return false if you want to prevent reactivity for this call/get */ /** @@ -59,7 +59,7 @@ export const make_reactive = (Entity, options) => { * @param {import('#client').Source} target * @param {TProperty} property * @param {TEntityInstance} entity_instance - * @param {TEntityInstance[TProperty] extends (...args: any)=>any ? Parameters: never} params + * @param {unknown[]} params */ function notify_if_required(target, property, entity_instance, ...params) { if (options.interceptors?.[property]?.(entity_instance, property, ...params) === false) { From 35d990614414a9acb766600cfc0d133abfc04382 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 8 May 2024 17:18:47 +0330 Subject: [PATCH 06/71] improved jsdocs for interceptors --- packages/svelte/src/reactivity/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index ad4c5a019fce..af8eb35938ea 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -42,7 +42,7 @@ function get_this() { * @template {(keyof TEntityInstance)[]} TMutationProperties * @typedef {object} Options * @prop {TMutationProperties} mutation_properties - an array of property names on `TEntityInstance`. when calling a property on `TEntityInstance`, if the property name exists in this array, then mentioned property causes reactivity. - * @prop {Interceptors} [interceptors={}] - if the property names in `mutation_properties` shouldn't be always reactive, such calling `set.add(2)` twice, you can prevent the reactivity by returning false from these interceptors + * @prop {Interceptors} [interceptors={}] - if the property names in `mutation_properties` shouldn't be always reactive, such calling `set.add(2)` twice, you can prevent the reactivity by returning `false` from these interceptors */ /** From 6162079f2602360bdf9ec92c3ea439f1ff56c681 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 8 May 2024 17:22:02 +0330 Subject: [PATCH 07/71] removed map from utils because it wasnt used anymore --- packages/svelte/src/reactivity/map.js | 178 ++---------------------- packages/svelte/src/reactivity/utils.js | 30 ---- 2 files changed, 14 insertions(+), 194 deletions(-) diff --git a/packages/svelte/src/reactivity/map.js b/packages/svelte/src/reactivity/map.js index ece3d1845d2b..5286f425bad8 100644 --- a/packages/svelte/src/reactivity/map.js +++ b/packages/svelte/src/reactivity/map.js @@ -1,166 +1,16 @@ -import { DEV } from 'esm-env'; -import { source, set } from '../internal/client/reactivity/sources.js'; -import { get } from '../internal/client/runtime.js'; -import { UNINITIALIZED } from '../constants.js'; - -/** - * @template K - * @template V - * @extends {Map} - */ -export class ReactiveMap extends Map { - /** @type {Map>} */ - #sources = new Map(); - #version = source(0); - #size = source(0); - - /** - * @param {Iterable | null | undefined} [value] - */ - constructor(value) { - super(); - - // If the value is invalid then the native exception will fire here - if (DEV) new Map(value); - - if (value) { - for (var [key, v] of value) { - super.set(key, v); - } - this.#size.v = super.size; - } - } - - #increment_version() { - set(this.#version, this.#version.v + 1); - } - - /** @param {K} key */ - has(key) { - var sources = this.#sources; - var s = sources.get(key); - - if (s === undefined) { - var ret = super.get(key); - if (ret !== undefined) { - s = source(Symbol()); - sources.set(key, s); - } else { - // We should always track the version in case - // the Set ever gets this value in the future. - get(this.#version); - return false; - } - } - - get(s); - return true; - } - - /** - * @param {(value: V, key: K, map: Map) => void} callbackfn - * @param {any} [this_arg] - */ - forEach(callbackfn, this_arg) { - get(this.#version); - - return super.forEach(callbackfn, this_arg); - } - - /** @param {K} key */ - get(key) { - var sources = this.#sources; - var s = sources.get(key); - - if (s === undefined) { - var ret = super.get(key); - if (ret !== undefined) { - s = source(Symbol()); - sources.set(key, s); - } else { - // We should always track the version in case - // the Set ever gets this value in the future. - get(this.#version); - return undefined; - } +import { make_reactive } from './utils.js'; + +export const ReactiveMap = make_reactive(Map, { + mutation_properties: ['clear', 'delete', 'set'], + interceptors: { + set: (value, property, ...params) => { + return value.get(params[0]) !== params[2] || params[2] !== undefined; + }, + clear: (value, property, ...params) => { + return value.size !== 0; + }, + delete: (value, property, ...params) => { + return value.has(params[0]); } - - get(s); - return super.get(key); - } - - /** - * @param {K} key - * @param {V} value - * */ - set(key, value) { - var sources = this.#sources; - var s = sources.get(key); - var prev_res = super.get(key); - var res = super.set(key, value); - - if (s === undefined) { - sources.set(key, source(Symbol())); - set(this.#size, super.size); - this.#increment_version(); - } else if (prev_res !== value) { - set(s, Symbol()); - } - - return res; - } - - /** @param {K} key */ - delete(key) { - var sources = this.#sources; - var s = sources.get(key); - var res = super.delete(key); - - if (s !== undefined) { - sources.delete(key); - set(this.#size, super.size); - set(s, UNINITIALIZED); - this.#increment_version(); - } - - return res; - } - - clear() { - var sources = this.#sources; - - if (super.size !== 0) { - set(this.#size, 0); - for (var s of sources.values()) { - set(s, UNINITIALIZED); - } - this.#increment_version(); - sources.clear(); - } - super.clear(); - } - - keys() { - get(this.#version); - return super.keys(); - } - - values() { - get(this.#version); - return super.values(); - } - - entries() { - get(this.#version); - return super.entries(); - } - - [Symbol.iterator]() { - return this.entries(); - } - - get size() { - get(this.#size); - return super.size; } -} +}); diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index af8eb35938ea..c2e1f984aac1 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -1,36 +1,6 @@ import { source, set } from '../internal/client/reactivity/sources.js'; import { get } from '../internal/client/runtime.js'; -/** - * @template T - * @template U - * @param {Iterable} iterable - * @param {(value: T) => U} fn - * @param {string} name - * @returns {IterableIterator} - */ -export function map(iterable, fn, name) { - return { - [Symbol.iterator]: get_this, - next() { - for (const value of iterable) { - return { done: false, value: fn(value) }; - } - - return { done: true, value: undefined }; - }, - // @ts-expect-error - get [Symbol.toStringTag]() { - return name; - } - }; -} - -/** @this {any} */ -function get_this() { - return this; -} - /** * @template TEntityInstance * @template {(keyof TEntityInstance)[]} TMutationProperties From 8da76fa8930132285ba24b959be61ef6dec7684e Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 8 May 2024 17:24:42 +0330 Subject: [PATCH 08/71] removed enforcement of defining interceptors since each mutation_property doesnt *require* one --- packages/svelte/src/reactivity/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index c2e1f984aac1..aa6bba016b77 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -4,7 +4,7 @@ import { get } from '../internal/client/runtime.js'; /** * @template TEntityInstance * @template {(keyof TEntityInstance)[]} TMutationProperties - * @typedef {Recordboolean>} Interceptors - return false if you want to prevent reactivity for this call/get + * @typedef {Partialboolean>>} Interceptors - return false if you want to prevent reactivity for this call/get */ /** From 6c4a4b61ac08473dee7a93e5114444009b4a76f2 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 8 May 2024 17:41:33 +0330 Subject: [PATCH 09/71] 1- interceptors can be used for anything now (types) 2- removed templated constructor params cause it didn't do anything (types) --- packages/svelte/src/reactivity/utils.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index aa6bba016b77..cda9fde6d56b 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -12,7 +12,7 @@ import { get } from '../internal/client/runtime.js'; * @template {(keyof TEntityInstance)[]} TMutationProperties * @typedef {object} Options * @prop {TMutationProperties} mutation_properties - an array of property names on `TEntityInstance`. when calling a property on `TEntityInstance`, if the property name exists in this array, then mentioned property causes reactivity. - * @prop {Interceptors} [interceptors={}] - if the property names in `mutation_properties` shouldn't be always reactive, such calling `set.add(2)` twice, you can prevent the reactivity by returning `false` from these interceptors + * @prop {Interceptors} [interceptors={}] - if the property names in `mutation_properties` shouldn't cause reactivity, such calling `set.add(2)` twice or accessing a property shouldn't be reactive based on some conditions, you can prevent the reactivity by returning `false` from these interceptors */ /** @@ -44,15 +44,11 @@ export const make_reactive = (Entity, options) => { } } - /** - * @template {ConstructorParameters} TParams - */ // we return a class so that the caller can call it with new // @ts-ignore return class { /** - * @param {TParams} params - * @returns + * @param {ConstructorParameters} params */ constructor(...params) { const sig = source(0); From 81d33e34658e17b45189773ce8d550d55a66291e Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 8 May 2024 23:06:49 +0330 Subject: [PATCH 10/71] renamed signal to version_signal --- packages/svelte/src/reactivity/utils.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index cda9fde6d56b..9bfdee9074fb 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -26,21 +26,21 @@ export const make_reactive = (Entity, options) => { /** * @template {InstanceType} TEntityInstance * @template {keyof TEntityInstance} TProperty - * @param {import('#client').Source} target + * @param {import('#client').Source} version_signal * @param {TProperty} property * @param {TEntityInstance} entity_instance * @param {unknown[]} params */ - function notify_if_required(target, property, entity_instance, ...params) { + function notify_if_required(version_signal, property, entity_instance, ...params) { if (options.interceptors?.[property]?.(entity_instance, property, ...params) === false) { // if interceptor said to not make this call reactive then bailout return; } if (options.mutation_properties.some((v) => v === property)) { - set(target, target.v + 1); + set(version_signal, version_signal.v + 1); } else { - get(target); + get(version_signal); } } @@ -51,24 +51,22 @@ export const make_reactive = (Entity, options) => { * @param {ConstructorParameters} params */ constructor(...params) { - const sig = source(0); - + const version_signal = source(0); return new Proxy(new Entity(...params), { get(target, property) { const orig_property = target[property]; - let result; if (typeof orig_property === 'function') { - // Bind functions directly to the Set + // Bind functions directly to the `TEntity` result = ((/** @type {any} */ ...params) => { - notify_if_required(sig, property, target, ...params); + notify_if_required(version_signal, property, target, ...params); return orig_property.bind(target)(...params); }).bind(target); } else { // Properly handle getters + notify_if_required(version_signal, property, target); result = Reflect.get(target, property, target); - notify_if_required(sig, property, target); } return result; From ea911924fd63a097a07e9d37f3f5dd5a0b0d5877 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Thu, 9 May 2024 15:42:29 +0330 Subject: [PATCH 11/71] added some comments --- packages/svelte/src/reactivity/utils.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 9bfdee9074fb..3c9c8b538a29 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -4,7 +4,7 @@ import { get } from '../internal/client/runtime.js'; /** * @template TEntityInstance * @template {(keyof TEntityInstance)[]} TMutationProperties - * @typedef {Partialboolean>>} Interceptors - return false if you want to prevent reactivity for this call/get + * @typedef {Partialboolean>>} Interceptors - return false if you want to prevent reactivity for this call, DO NOT USE INTERCEPTORS FOR READ METHODS */ /** @@ -38,9 +38,9 @@ export const make_reactive = (Entity, options) => { } if (options.mutation_properties.some((v) => v === property)) { - set(version_signal, version_signal.v + 1); + set(version_signal, version_signal.v + 1); // write methods } else { - get(version_signal); + get(version_signal); // read methods } } From 6885f1cfd2435a4f4cb8fe091f84838cf209b66f Mon Sep 17 00:00:00 2001 From: godzylinux Date: Thu, 9 May 2024 18:27:57 +0330 Subject: [PATCH 12/71] again another take on this matter, but this version is not optimized at all --- packages/svelte/src/reactivity/set.js | 28 ++++-- packages/svelte/src/reactivity/utils.js | 121 +++++++++++++++++++++--- 2 files changed, 128 insertions(+), 21 deletions(-) diff --git a/packages/svelte/src/reactivity/set.js b/packages/svelte/src/reactivity/set.js index a98302e3e3f4..9daad2718867 100644 --- a/packages/svelte/src/reactivity/set.js +++ b/packages/svelte/src/reactivity/set.js @@ -2,15 +2,31 @@ import { make_reactive } from './utils.js'; export const ReactiveSet = make_reactive(Set, { mutation_properties: ['add', 'clear', 'delete'], + read_properties: ['has', 'size'], interceptors: { - add: (value, property, ...params) => { - return !value.has(params[0]); + add: (notify_read_methods, value, property, ...params) => { + if (value.has(params[0])) { + return false; + } + notify_read_methods(['has'], params[0]); + notify_read_methods(['size']); + return true; }, - clear: (value, property, ...params) => { - return value.size !== 0; + clear: (notify_read_methods, value, property, ...params) => { + if (value.size == 0) { + return false; + } + notify_read_methods(['has'], params[0]); + notify_read_methods(['size']); + return true; }, - delete: (value, property, ...params) => { - return value.has(params[0]); + delete: (notify_read_methods, value, property, ...params) => { + if (!value.has(params[0])) { + return false; + } + notify_read_methods(['has'], params[0]); + notify_read_methods(['size']); + return true; } } }); diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 3c9c8b538a29..df61c258c874 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -1,46 +1,88 @@ +import { DEV } from 'esm-env'; import { source, set } from '../internal/client/reactivity/sources.js'; import { get } from '../internal/client/runtime.js'; /** * @template TEntityInstance * @template {(keyof TEntityInstance)[]} TMutationProperties - * @typedef {Partialboolean>>} Interceptors - return false if you want to prevent reactivity for this call, DO NOT USE INTERCEPTORS FOR READ METHODS + * @template {(keyof TEntityInstance)[]} TReadProperties + * @typedef {Partialvoid ,value: TEntityInstance, property: TMutationProperties[number], ...params: unknown[])=>boolean>>} Interceptors - return false if you want to prevent reactivity for this call, DO NOT USE INTERCEPTORS FOR READ METHODS */ /** * @template TEntityInstance * @template {(keyof TEntityInstance)[]} TMutationProperties + * @template {(keyof TEntityInstance)[]} TReadProperties * @typedef {object} Options - * @prop {TMutationProperties} mutation_properties - an array of property names on `TEntityInstance`. when calling a property on `TEntityInstance`, if the property name exists in this array, then mentioned property causes reactivity. - * @prop {Interceptors} [interceptors={}] - if the property names in `mutation_properties` shouldn't cause reactivity, such calling `set.add(2)` twice or accessing a property shouldn't be reactive based on some conditions, you can prevent the reactivity by returning `false` from these interceptors + * @prop {TMutationProperties} mutation_properties - an array of property names on `TEntityInstance` that when calling a property on `TEntityInstance`, if the property name exists in this array, then mentioned property causes reactivity. + * @prop {TReadProperties} read_properties - an array of property names on `TEntityInstance` that `mutation_properties` affects + * @prop {Interceptors} [interceptors={}] - if the property names in `mutation_properties` shouldn't cause reactivity, such calling `set.add(2)` twice or accessing a property shouldn't be reactive based on some conditions, you can prevent the reactivity by returning `false` from these interceptors */ +/** @typedef {Map>>} ReadMethodsSignals */ + /** * @template {new (...args: any) => any} TEntity - the entity we want to make reactive * @template {(keyof InstanceType)[]} TMutationProperties + * @template {(keyof InstanceType)[]} TReadProperties * @param {TEntity} Entity - the class/function we want to make reactive - * @param {Options, TMutationProperties>} options - configurations for how reactivity works for this entity + * @param {Options, TMutationProperties, TReadProperties>} options - configurations for how reactivity works for this entity * @returns {TEntity} */ export const make_reactive = (Entity, options) => { /** * @template {InstanceType} TEntityInstance * @template {keyof TEntityInstance} TProperty - * @param {import('#client').Source} version_signal + * @param {import('#client').Source} version_signal + * @param {ReadMethodsSignals} read_methods_signals * @param {TProperty} property * @param {TEntityInstance} entity_instance * @param {unknown[]} params */ - function notify_if_required(version_signal, property, entity_instance, ...params) { - if (options.interceptors?.[property]?.(entity_instance, property, ...params) === false) { - // if interceptor said to not make this call reactive then bailout - return; + function notify_if_required( + version_signal, + read_methods_signals, + property, + entity_instance, + ...params + ) { + /** + * @param {TReadProperties} method_names + * @param {unknown} param + */ + function notify_read_methods(method_names, param) { + method_names.forEach((name) => { + if (DEV && !options.read_properties.includes(name)) { + throw new Error( + `when trying to notify reactions got a read method that wasn't defined in options: ${name.toString()}` + ); + } + const sig = get_signal_for_function(read_methods_signals, name, param); + increment_signal(version_signal, sig); + }); } + if ( + options.interceptors?.[property]?.( + notify_read_methods, + entity_instance, + property, + ...params + ) === false + ) { + return; + } if (options.mutation_properties.some((v) => v === property)) { - set(version_signal, version_signal.v + 1); // write methods + increment_signal(version_signal); } else { - get(version_signal); // read methods + if (options.read_properties.includes(property)) { + params.forEach((param) => { + const sig = get_signal_for_function(read_methods_signals, property, param); + get(sig); + }); + } else { + get(version_signal); + } } } @@ -51,7 +93,10 @@ export const make_reactive = (Entity, options) => { * @param {ConstructorParameters} params */ constructor(...params) { - const version_signal = source(0); + /** @type {Map>>} */ + // TODO: comment what read_methods_signals and version_signal do + const read_methods_signals = new Map(); + const version_signal = source(false); return new Proxy(new Entity(...params), { get(target, property) { const orig_property = target[property]; @@ -59,13 +104,13 @@ export const make_reactive = (Entity, options) => { if (typeof orig_property === 'function') { // Bind functions directly to the `TEntity` - result = ((/** @type {any} */ ...params) => { - notify_if_required(version_signal, property, target, ...params); + result = ((/** @type {unknown[]} */ ...params) => { + notify_if_required(version_signal, read_methods_signals, property, target, ...params); return orig_property.bind(target)(...params); }).bind(target); } else { // Properly handle getters - notify_if_required(version_signal, property, target); + notify_if_required(version_signal, read_methods_signals, property, target); result = Reflect.get(target, property, target); } @@ -75,3 +120,49 @@ export const make_reactive = (Entity, options) => { } }; }; + +/** + * gets the signal for this function based on params. If the signal doesn't exist, it creates a new one and returns that + * @param {ReadMethodsSignals} signals_map + * @param {string | symbol | number} function_name + * @param {unknown} param + */ +const get_signal_for_function = (signals_map, function_name, param) => { + /** + * @type {Map>} + */ + let params_to_signal_map; + if (!signals_map.has(function_name)) { + params_to_signal_map = new Map([[param, source(false)]]); + signals_map.set(function_name, params_to_signal_map); + } else { + params_to_signal_map = /** + * @type {Map>} + */ (signals_map.get(function_name)); + } + + /** + * @type {import("#client").Source>} + */ + let signal; + if (!params_to_signal_map.has(param)) { + signal = source(false); + params_to_signal_map.set(param, signal); + } else { + signal = /** + * @type {import("#client").Source} + */ (params_to_signal_map.get(param)); + } + + return signal; +}; + +/** + * toggles the signal value. this change notifies any reactions (not using number explicitly cause its not required, change from true to false or vice versa is enough). + * @param {import("#client").Source} version_signal + * @param {import("#client").Source} [function_signal] + */ +const increment_signal = (version_signal, function_signal) => { + set(version_signal, !version_signal.v); + function_signal && set(function_signal, !function_signal.v); +}; From 3dc711da165a653bd89af0cb514a1472f056df66 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Thu, 9 May 2024 18:35:42 +0330 Subject: [PATCH 13/71] added comments to read_methods_signals and version_signal --- packages/svelte/src/reactivity/utils.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index df61c258c874..0b123a17d491 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -93,9 +93,18 @@ export const make_reactive = (Entity, options) => { * @param {ConstructorParameters} params */ constructor(...params) { - /** @type {Map>>} */ - // TODO: comment what read_methods_signals and version_signal do + /** + * each read method can be tracked like has, size, get and etc. these props might depend on a parameter. they have to reactive based on the + * parameter they depend on, if it requires a change. for instance if you call `set.has(2)` then call `set.add(5)` the former shouldn't get notified. + * so what is do we need to store if this needs to be generic? function name + parameter. unfortunately for each parameter we need a new signal, why? + * because arrays don't equal themselves even if they have the same value (referential equality). + * fortunately current builtins that we try to make reactive only take 1 parameter for their read methods so this is fine. + * @type {Map>>} + **/ const read_methods_signals = new Map(); + /** + * other props that get notified based on any change listen to version + */ const version_signal = source(false); return new Proxy(new Entity(...params), { get(target, property) { From 785e51ea649baaf850e34c25e576ece1934642f9 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Thu, 9 May 2024 18:54:37 +0330 Subject: [PATCH 14/71] fixed a bug where if a method didn't take arguments reactivity wouldn't work --- packages/svelte/src/reactivity/utils.js | 28 +++++++++++++------------ 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 0b123a17d491..070fc15cf23c 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -4,29 +4,29 @@ import { get } from '../internal/client/runtime.js'; /** * @template TEntityInstance - * @template {(keyof TEntityInstance)[]} TMutationProperties + * @template {(keyof TEntityInstance)[]} TWriteProperties * @template {(keyof TEntityInstance)[]} TReadProperties - * @typedef {Partialvoid ,value: TEntityInstance, property: TMutationProperties[number], ...params: unknown[])=>boolean>>} Interceptors - return false if you want to prevent reactivity for this call, DO NOT USE INTERCEPTORS FOR READ METHODS + * @typedef {Partialvoid ,value: TEntityInstance, property: TWriteProperties[number], ...params: unknown[])=>boolean>>} Interceptors - return false if you want to prevent reactivity for this call, DO NOT USE INTERCEPTORS FOR READ METHODS */ /** * @template TEntityInstance - * @template {(keyof TEntityInstance)[]} TMutationProperties + * @template {(keyof TEntityInstance)[]} TWriteProperties * @template {(keyof TEntityInstance)[]} TReadProperties * @typedef {object} Options - * @prop {TMutationProperties} mutation_properties - an array of property names on `TEntityInstance` that when calling a property on `TEntityInstance`, if the property name exists in this array, then mentioned property causes reactivity. + * @prop {TWriteProperties} write_properties - an array of property names on `TEntityInstance` that when calling a property on `TEntityInstance`, if the property name exists in this array, then mentioned property causes reactivity. * @prop {TReadProperties} read_properties - an array of property names on `TEntityInstance` that `mutation_properties` affects - * @prop {Interceptors} [interceptors={}] - if the property names in `mutation_properties` shouldn't cause reactivity, such calling `set.add(2)` twice or accessing a property shouldn't be reactive based on some conditions, you can prevent the reactivity by returning `false` from these interceptors + * @prop {Interceptors} [interceptors={}] - if the property names in `mutation_properties` shouldn't cause reactivity, such calling `set.add(2)` twice or accessing a property shouldn't be reactive based on some conditions, you can prevent the reactivity by returning `false` from these interceptors */ /** @typedef {Map>>} ReadMethodsSignals */ /** * @template {new (...args: any) => any} TEntity - the entity we want to make reactive - * @template {(keyof InstanceType)[]} TMutationProperties + * @template {(keyof InstanceType)[]} TWriteProperties * @template {(keyof InstanceType)[]} TReadProperties * @param {TEntity} Entity - the class/function we want to make reactive - * @param {Options, TMutationProperties, TReadProperties>} options - configurations for how reactivity works for this entity + * @param {Options, TWriteProperties, TReadProperties>} options - configurations for how reactivity works for this entity * @returns {TEntity} */ export const make_reactive = (Entity, options) => { @@ -48,17 +48,19 @@ export const make_reactive = (Entity, options) => { ) { /** * @param {TReadProperties} method_names - * @param {unknown} param + * @param {unknown[]} params */ - function notify_read_methods(method_names, param) { + function notify_read_methods(method_names, ...params) { method_names.forEach((name) => { if (DEV && !options.read_properties.includes(name)) { throw new Error( `when trying to notify reactions got a read method that wasn't defined in options: ${name.toString()}` ); } - const sig = get_signal_for_function(read_methods_signals, name, param); - increment_signal(version_signal, sig); + (params.length == 0 ? [null] : params).forEach((param) => { + const sig = get_signal_for_function(read_methods_signals, name, param); + increment_signal(version_signal, sig); + }); }); } @@ -72,11 +74,11 @@ export const make_reactive = (Entity, options) => { ) { return; } - if (options.mutation_properties.some((v) => v === property)) { + if (options.write_properties.some((v) => v === property)) { increment_signal(version_signal); } else { if (options.read_properties.includes(property)) { - params.forEach((param) => { + (params.length == 0 ? [null] : params).forEach((param) => { const sig = get_signal_for_function(read_methods_signals, property, param); get(sig); }); From 0df3dcd92508408f8c11bb6d85532592fa95fd26 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Thu, 9 May 2024 18:56:53 +0330 Subject: [PATCH 15/71] fixed test because logging false after clear for 3 is not fine-grained (because we already got it after delete) --- packages/svelte/src/reactivity/set.js | 2 +- packages/svelte/src/reactivity/set.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/reactivity/set.js b/packages/svelte/src/reactivity/set.js index 9daad2718867..81c1077e6d85 100644 --- a/packages/svelte/src/reactivity/set.js +++ b/packages/svelte/src/reactivity/set.js @@ -1,7 +1,7 @@ import { make_reactive } from './utils.js'; export const ReactiveSet = make_reactive(Set, { - mutation_properties: ['add', 'clear', 'delete'], + write_properties: ['add', 'clear', 'delete'], read_properties: ['has', 'size'], interceptors: { add: (notify_read_methods, value, property, ...params) => { diff --git a/packages/svelte/src/reactivity/set.test.ts b/packages/svelte/src/reactivity/set.test.ts index 97d0869c9e66..307ddd3bac71 100644 --- a/packages/svelte/src/reactivity/set.test.ts +++ b/packages/svelte/src/reactivity/set.test.ts @@ -30,7 +30,7 @@ test('set.values()', () => { set.clear(); }); - assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, false, []]); + assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, []]); cleanup(); }); From 2177272082c63611ee2eb7247b26e06f7018fcb4 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 10 May 2024 01:10:03 +0330 Subject: [PATCH 16/71] initial map support --- packages/svelte/src/reactivity/map.js | 32 +++++++++++++++++++------ packages/svelte/src/reactivity/utils.js | 8 +++---- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/packages/svelte/src/reactivity/map.js b/packages/svelte/src/reactivity/map.js index 5286f425bad8..22a5e0787c15 100644 --- a/packages/svelte/src/reactivity/map.js +++ b/packages/svelte/src/reactivity/map.js @@ -1,16 +1,34 @@ import { make_reactive } from './utils.js'; export const ReactiveMap = make_reactive(Map, { - mutation_properties: ['clear', 'delete', 'set'], + write_properties: ['clear', 'delete', 'set'], + read_properties: ['get', 'keys', 'size', 'entries', 'values'], interceptors: { - set: (value, property, ...params) => { - return value.get(params[0]) !== params[2] || params[2] !== undefined; + set: (notify_read_methods, value, property, ...params) => { + if (value.get(params[0]) === params[1] && params[1] !== undefined) { + return false; + } + if (!value.has(params[0])) { + notify_read_methods(['keys', 'size']); + } + notify_read_methods(['entries', 'values']); + notify_read_methods(['get'], params[1]); + return true; }, - clear: (value, property, ...params) => { - return value.size !== 0; + clear: (notify_read_methods, value, property, ...params) => { + if (value.size === 0) { + return false; + } + notify_read_methods(['keys', 'size', 'values', 'entries']); + return true; }, - delete: (value, property, ...params) => { - return value.has(params[0]); + delete: (notify_read_methods, value, property, ...params) => { + if (!value.has(params[0])) { + return false; + } + notify_read_methods(['get'], value.get(params[0])); + notify_read_methods(['keys', 'size', 'values', 'entries']); + return true; } } }); diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 070fc15cf23c..e775d1eefe25 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -153,7 +153,7 @@ const get_signal_for_function = (signals_map, function_name, param) => { } /** - * @type {import("#client").Source>} + * @type {import("#client").Source} */ let signal; if (!params_to_signal_map.has(param)) { @@ -171,9 +171,9 @@ const get_signal_for_function = (signals_map, function_name, param) => { /** * toggles the signal value. this change notifies any reactions (not using number explicitly cause its not required, change from true to false or vice versa is enough). * @param {import("#client").Source} version_signal - * @param {import("#client").Source} [function_signal] + * @param {import("#client").Source} [read_method_signal] */ -const increment_signal = (version_signal, function_signal) => { +const increment_signal = (version_signal, read_method_signal) => { set(version_signal, !version_signal.v); - function_signal && set(function_signal, !function_signal.v); + read_method_signal && set(read_method_signal, !read_method_signal.v); }; From cb8bf445921364cfd1f256b42b2d36226fcbc573 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 10 May 2024 01:22:49 +0330 Subject: [PATCH 17/71] instead of nested functions used separate function --- packages/svelte/src/reactivity/utils.js | 153 ++++++++++++++---------- 1 file changed, 93 insertions(+), 60 deletions(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index e775d1eefe25..9e73becbcde5 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -30,64 +30,6 @@ import { get } from '../internal/client/runtime.js'; * @returns {TEntity} */ export const make_reactive = (Entity, options) => { - /** - * @template {InstanceType} TEntityInstance - * @template {keyof TEntityInstance} TProperty - * @param {import('#client').Source} version_signal - * @param {ReadMethodsSignals} read_methods_signals - * @param {TProperty} property - * @param {TEntityInstance} entity_instance - * @param {unknown[]} params - */ - function notify_if_required( - version_signal, - read_methods_signals, - property, - entity_instance, - ...params - ) { - /** - * @param {TReadProperties} method_names - * @param {unknown[]} params - */ - function notify_read_methods(method_names, ...params) { - method_names.forEach((name) => { - if (DEV && !options.read_properties.includes(name)) { - throw new Error( - `when trying to notify reactions got a read method that wasn't defined in options: ${name.toString()}` - ); - } - (params.length == 0 ? [null] : params).forEach((param) => { - const sig = get_signal_for_function(read_methods_signals, name, param); - increment_signal(version_signal, sig); - }); - }); - } - - if ( - options.interceptors?.[property]?.( - notify_read_methods, - entity_instance, - property, - ...params - ) === false - ) { - return; - } - if (options.write_properties.some((v) => v === property)) { - increment_signal(version_signal); - } else { - if (options.read_properties.includes(property)) { - (params.length == 0 ? [null] : params).forEach((param) => { - const sig = get_signal_for_function(read_methods_signals, property, param); - get(sig); - }); - } else { - get(version_signal); - } - } - } - // we return a class so that the caller can call it with new // @ts-ignore return class { @@ -116,12 +58,19 @@ export const make_reactive = (Entity, options) => { if (typeof orig_property === 'function') { // Bind functions directly to the `TEntity` result = ((/** @type {unknown[]} */ ...params) => { - notify_if_required(version_signal, read_methods_signals, property, target, ...params); + notify_if_required( + version_signal, + read_methods_signals, + property, + target, + options, + ...params + ); return orig_property.bind(target)(...params); }).bind(target); } else { // Properly handle getters - notify_if_required(version_signal, read_methods_signals, property, target); + notify_if_required(version_signal, read_methods_signals, property, target, options); result = Reflect.get(target, property, target); } @@ -132,6 +81,90 @@ export const make_reactive = (Entity, options) => { }; }; +/** + * @template {new (...args: any) => any} TEntity + * @template {(keyof TEntityInstance)[]} TWriteProperties + * @template {(keyof TEntityInstance)[]} TReadProperties + * @template {InstanceType} TEntityInstance + * @template {keyof TEntityInstance} TProperty + * @param {import('#client').Source} version_signal + * @param {ReadMethodsSignals} read_methods_signals + * @param {TProperty} property + * @param {TEntityInstance} entity_instance + * @param {Options, TWriteProperties, TReadProperties>} options + * @param {unknown[]} params + */ +function notify_if_required( + version_signal, + read_methods_signals, + property, + entity_instance, + options, + ...params +) { + if ( + options.interceptors?.[property]?.( + (methods, ...params) => { + return notify_read_methods( + options, + version_signal, + read_methods_signals, + methods, + ...params + ); + }, + entity_instance, + property, + ...params + ) === false + ) { + return; + } + if (options.write_properties.some((v) => v === property)) { + increment_signal(version_signal); + } else { + if (options.read_properties.includes(property)) { + (params.length == 0 ? [null] : params).forEach((param) => { + const sig = get_signal_for_function(read_methods_signals, property, param); + get(sig); + }); + } else { + get(version_signal); + } + } +} + +/** + * @template {new (...args: any) => any} TEntity + * @template {(keyof TEntityInstance)[]} TWriteProperties + * @template {(keyof TEntityInstance)[]} TReadProperties + * @template {InstanceType} TEntityInstance + * @param {import('#client').Source} version_signal + * @param {ReadMethodsSignals} read_methods_signals + * @param {Options, TWriteProperties, TReadProperties>} options + * @param {TReadProperties} method_names + * @param {unknown[]} params + */ +function notify_read_methods( + options, + version_signal, + read_methods_signals, + method_names, + ...params +) { + method_names.forEach((name) => { + if (DEV && !options.read_properties.includes(name)) { + throw new Error( + `when trying to notify reactions got a read method that wasn't defined in options: ${name.toString()}` + ); + } + (params.length == 0 ? [null] : params).forEach((param) => { + const sig = get_signal_for_function(read_methods_signals, name, param); + increment_signal(version_signal, sig); + }); + }); +} + /** * gets the signal for this function based on params. If the signal doesn't exist, it creates a new one and returns that * @param {ReadMethodsSignals} signals_map From 80a73b43653afa87261999a68080c6eac1fc40ad Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 10 May 2024 02:06:51 +0330 Subject: [PATCH 18/71] added the ability to notify with all params --- packages/svelte/src/reactivity/map.js | 12 ++++++------ packages/svelte/src/reactivity/set.js | 4 ++-- packages/svelte/src/reactivity/utils.js | 18 +++++++++++++----- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/packages/svelte/src/reactivity/map.js b/packages/svelte/src/reactivity/map.js index 22a5e0787c15..18a0b1de1bf7 100644 --- a/packages/svelte/src/reactivity/map.js +++ b/packages/svelte/src/reactivity/map.js @@ -1,32 +1,32 @@ -import { make_reactive } from './utils.js'; +import { make_reactive, NOTIFY_WITH_ALL_PARAMS } from './utils.js'; export const ReactiveMap = make_reactive(Map, { write_properties: ['clear', 'delete', 'set'], - read_properties: ['get', 'keys', 'size', 'entries', 'values'], + read_properties: ['get', 'keys', 'size', 'entries', 'values', 'has'], interceptors: { set: (notify_read_methods, value, property, ...params) => { - if (value.get(params[0]) === params[1] && params[1] !== undefined) { + if (value.get(params[0]) === params[1]) { return false; } if (!value.has(params[0])) { notify_read_methods(['keys', 'size']); } notify_read_methods(['entries', 'values']); - notify_read_methods(['get'], params[1]); + notify_read_methods(['get', 'has'], params[1]); return true; }, clear: (notify_read_methods, value, property, ...params) => { if (value.size === 0) { return false; } - notify_read_methods(['keys', 'size', 'values', 'entries']); + notify_read_methods(['keys', 'size', 'values', 'entries', 'has'], NOTIFY_WITH_ALL_PARAMS); return true; }, delete: (notify_read_methods, value, property, ...params) => { if (!value.has(params[0])) { return false; } - notify_read_methods(['get'], value.get(params[0])); + notify_read_methods(['get', 'has'], params[0]); notify_read_methods(['keys', 'size', 'values', 'entries']); return true; } diff --git a/packages/svelte/src/reactivity/set.js b/packages/svelte/src/reactivity/set.js index 81c1077e6d85..532c85ec006d 100644 --- a/packages/svelte/src/reactivity/set.js +++ b/packages/svelte/src/reactivity/set.js @@ -1,4 +1,4 @@ -import { make_reactive } from './utils.js'; +import { make_reactive, NOTIFY_WITH_ALL_PARAMS } from './utils.js'; export const ReactiveSet = make_reactive(Set, { write_properties: ['add', 'clear', 'delete'], @@ -16,7 +16,7 @@ export const ReactiveSet = make_reactive(Set, { if (value.size == 0) { return false; } - notify_read_methods(['has'], params[0]); + notify_read_methods(['has'], NOTIFY_WITH_ALL_PARAMS); notify_read_methods(['size']); return true; }, diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 9e73becbcde5..fce6e5b530ae 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -2,6 +2,8 @@ import { DEV } from 'esm-env'; import { source, set } from '../internal/client/reactivity/sources.js'; import { get } from '../internal/client/runtime.js'; +export const NOTIFY_WITH_ALL_PARAMS = `${crypto?.randomUUID?.() ?? Date.now().toString() + Date.now().toString()}`; + /** * @template TEntityInstance * @template {(keyof TEntityInstance)[]} TWriteProperties @@ -143,7 +145,7 @@ function notify_if_required( * @param {ReadMethodsSignals} read_methods_signals * @param {Options, TWriteProperties, TReadProperties>} options * @param {TReadProperties} method_names - * @param {unknown[]} params + * @param {unknown[]} params - if you want to notify for all parameters pass NOTIFY_WITH_ALL_PARAMS constant, for instance some methods like `clear` should notify all `something.get(x)` methods; on these cases set this flag to true */ function notify_read_methods( options, @@ -158,10 +160,16 @@ function notify_read_methods( `when trying to notify reactions got a read method that wasn't defined in options: ${name.toString()}` ); } - (params.length == 0 ? [null] : params).forEach((param) => { - const sig = get_signal_for_function(read_methods_signals, name, param); - increment_signal(version_signal, sig); - }); + if (params.length == 1 && params[0] == NOTIFY_WITH_ALL_PARAMS) { + read_methods_signals.get(name)?.forEach((sig) => { + increment_signal(version_signal, sig); + }); + } else { + (params.length == 0 ? [null] : params).forEach((param) => { + const sig = get_signal_for_function(read_methods_signals, name, param); + increment_signal(version_signal, sig); + }); + } }); } From eaa3fc2d96cbd546906cde0503e5f7d342afeba4 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 10 May 2024 02:09:04 +0330 Subject: [PATCH 19/71] fixed test --- packages/svelte/src/reactivity/set.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/reactivity/set.test.ts b/packages/svelte/src/reactivity/set.test.ts index 307ddd3bac71..97d0869c9e66 100644 --- a/packages/svelte/src/reactivity/set.test.ts +++ b/packages/svelte/src/reactivity/set.test.ts @@ -30,7 +30,7 @@ test('set.values()', () => { set.clear(); }); - assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, []]); + assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, false, []]); cleanup(); }); From 2ad0a21796a1bfa6d4a1f79a16c4fbf949cbbe3d Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 10 May 2024 21:13:07 +0330 Subject: [PATCH 20/71] used symbol instead of stupid uuid! what was I thinking?? --- packages/svelte/src/reactivity/utils.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index fce6e5b530ae..5e05982926a6 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -2,7 +2,8 @@ import { DEV } from 'esm-env'; import { source, set } from '../internal/client/reactivity/sources.js'; import { get } from '../internal/client/runtime.js'; -export const NOTIFY_WITH_ALL_PARAMS = `${crypto?.randomUUID?.() ?? Date.now().toString() + Date.now().toString()}`; +// TODO: should this be in constants? +export const NOTIFY_WITH_ALL_PARAMS = Symbol(); /** * @template TEntityInstance From 1bfdd60c8b64f58f1307fdb86221ddb043f65fc2 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 10 May 2024 21:48:18 +0330 Subject: [PATCH 21/71] optimized when we create the signals --- packages/svelte/src/reactivity/utils.js | 28 ++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 5e05982926a6..0c5bbcce12dc 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -128,7 +128,8 @@ function notify_if_required( } else { if (options.read_properties.includes(property)) { (params.length == 0 ? [null] : params).forEach((param) => { - const sig = get_signal_for_function(read_methods_signals, property, param); + // read like methods should create the signal so what on any change to them they get notified + const sig = get_signal_for_function(read_methods_signals, property, param, true); get(sig); }); } else { @@ -168,24 +169,37 @@ function notify_read_methods( } else { (params.length == 0 ? [null] : params).forEach((param) => { const sig = get_signal_for_function(read_methods_signals, name, param); - increment_signal(version_signal, sig); + sig && increment_signal(version_signal, sig); }); } }); } /** - * gets the signal for this function based on params. If the signal doesn't exist, it creates a new one and returns that + * gets the signal for this function based on params. If the signal doesn't exist and create_signal_if_doesnt_exist is not set to true, it creates a new one and returns that + * @template {boolean} [TCreateSignalIfDoesntExist=false] * @param {ReadMethodsSignals} signals_map * @param {string | symbol | number} function_name * @param {unknown} param + * @param {TCreateSignalIfDoesntExist} [create_signal_if_doesnt_exist=false] + * @returns {TCreateSignalIfDoesntExist extends true ? import("#client").Source : import("#client").Source | null } */ -const get_signal_for_function = (signals_map, function_name, param) => { +const get_signal_for_function = ( + signals_map, + function_name, + param, + // @ts-ignore: this should be supported in jsdoc based on https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html#template but doesn't? + create_signal_if_doesnt_exist = false +) => { /** * @type {Map>} */ let params_to_signal_map; if (!signals_map.has(function_name)) { + if (!create_signal_if_doesnt_exist) { + // @ts-ignore + return null; + } params_to_signal_map = new Map([[param, source(false)]]); signals_map.set(function_name, params_to_signal_map); } else { @@ -199,6 +213,10 @@ const get_signal_for_function = (signals_map, function_name, param) => { */ let signal; if (!params_to_signal_map.has(param)) { + if (!create_signal_if_doesnt_exist) { + // @ts-ignore + return null; + } signal = source(false); params_to_signal_map.set(param, signal); } else { @@ -206,7 +224,7 @@ const get_signal_for_function = (signals_map, function_name, param) => { * @type {import("#client").Source} */ (params_to_signal_map.get(param)); } - + // @ts-ignore return signal; }; From c6c4f4648caa764e902d42d1d2ae43ed061d97d3 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 10 May 2024 21:58:35 +0330 Subject: [PATCH 22/71] added tests regarding handling of values not the set/map --- packages/svelte/src/reactivity/map.test.ts | 6 +----- packages/svelte/src/reactivity/set.test.ts | 10 +--------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/packages/svelte/src/reactivity/map.test.ts b/packages/svelte/src/reactivity/map.test.ts index 3cdba4bb38e5..f35b1a399a0f 100644 --- a/packages/svelte/src/reactivity/map.test.ts +++ b/packages/svelte/src/reactivity/map.test.ts @@ -207,11 +207,7 @@ test('not invoking reactivity when value is not in the map after changes', () => }); }); - assert.deepEqual(log, [1, undefined, undefined, undefined, 1, undefined]); + assert.deepEqual(log, [1, undefined, undefined, 1]); cleanup(); }); - -test('Map.instanceOf', () => { - assert.equal(new ReactiveMap() instanceof Map, true); -}); diff --git a/packages/svelte/src/reactivity/set.test.ts b/packages/svelte/src/reactivity/set.test.ts index 97d0869c9e66..811a99d525c7 100644 --- a/packages/svelte/src/reactivity/set.test.ts +++ b/packages/svelte/src/reactivity/set.test.ts @@ -143,17 +143,9 @@ test('not invoking reactivity when value is not in the set after changes', () => false, 'has 2', false, - 'has 3', - false, 'has 2', - true, - 'has 3', - false + true ]); cleanup(); }); - -test('Set.instanceOf', () => { - assert.equal(new ReactiveSet() instanceof Set, true); -}); From 68dfe7914a5f7d00699d28be9f8e5d3c14acc294 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Sat, 11 May 2024 10:18:06 +0330 Subject: [PATCH 23/71] removed size from read properties because it can depend on version signal and doesn't require its own signal --- packages/svelte/src/reactivity/map.js | 8 ++++---- packages/svelte/src/reactivity/set.js | 5 +---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/svelte/src/reactivity/map.js b/packages/svelte/src/reactivity/map.js index 18a0b1de1bf7..7ed7d01df9aa 100644 --- a/packages/svelte/src/reactivity/map.js +++ b/packages/svelte/src/reactivity/map.js @@ -2,14 +2,14 @@ import { make_reactive, NOTIFY_WITH_ALL_PARAMS } from './utils.js'; export const ReactiveMap = make_reactive(Map, { write_properties: ['clear', 'delete', 'set'], - read_properties: ['get', 'keys', 'size', 'entries', 'values', 'has'], + read_properties: ['get', 'keys', 'entries', 'values', 'has'], interceptors: { set: (notify_read_methods, value, property, ...params) => { if (value.get(params[0]) === params[1]) { return false; } if (!value.has(params[0])) { - notify_read_methods(['keys', 'size']); + notify_read_methods(['keys']); } notify_read_methods(['entries', 'values']); notify_read_methods(['get', 'has'], params[1]); @@ -19,7 +19,7 @@ export const ReactiveMap = make_reactive(Map, { if (value.size === 0) { return false; } - notify_read_methods(['keys', 'size', 'values', 'entries', 'has'], NOTIFY_WITH_ALL_PARAMS); + notify_read_methods(['keys', 'values', 'entries', 'has'], NOTIFY_WITH_ALL_PARAMS); return true; }, delete: (notify_read_methods, value, property, ...params) => { @@ -27,7 +27,7 @@ export const ReactiveMap = make_reactive(Map, { return false; } notify_read_methods(['get', 'has'], params[0]); - notify_read_methods(['keys', 'size', 'values', 'entries']); + notify_read_methods(['keys', 'values', 'entries']); return true; } } diff --git a/packages/svelte/src/reactivity/set.js b/packages/svelte/src/reactivity/set.js index 532c85ec006d..52fc70decb0f 100644 --- a/packages/svelte/src/reactivity/set.js +++ b/packages/svelte/src/reactivity/set.js @@ -2,14 +2,13 @@ import { make_reactive, NOTIFY_WITH_ALL_PARAMS } from './utils.js'; export const ReactiveSet = make_reactive(Set, { write_properties: ['add', 'clear', 'delete'], - read_properties: ['has', 'size'], + read_properties: ['has'], interceptors: { add: (notify_read_methods, value, property, ...params) => { if (value.has(params[0])) { return false; } notify_read_methods(['has'], params[0]); - notify_read_methods(['size']); return true; }, clear: (notify_read_methods, value, property, ...params) => { @@ -17,7 +16,6 @@ export const ReactiveSet = make_reactive(Set, { return false; } notify_read_methods(['has'], NOTIFY_WITH_ALL_PARAMS); - notify_read_methods(['size']); return true; }, delete: (notify_read_methods, value, property, ...params) => { @@ -25,7 +23,6 @@ export const ReactiveSet = make_reactive(Set, { return false; } notify_read_methods(['has'], params[0]); - notify_read_methods(['size']); return true; } } From b7049d889b2ec1856315fca398ed113adf05164b Mon Sep 17 00:00:00 2001 From: godzylinux Date: Sun, 12 May 2024 20:37:25 +0330 Subject: [PATCH 24/71] removed todo comment --- packages/svelte/src/reactivity/utils.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 0c5bbcce12dc..adfcda24f30d 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -2,7 +2,6 @@ import { DEV } from 'esm-env'; import { source, set } from '../internal/client/reactivity/sources.js'; import { get } from '../internal/client/runtime.js'; -// TODO: should this be in constants? export const NOTIFY_WITH_ALL_PARAMS = Symbol(); /** From 5950d47b363b13fb84d0655067404e474fda7b15 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 17 May 2024 10:23:10 +0330 Subject: [PATCH 25/71] improved comments --- packages/svelte/src/reactivity/utils.js | 32 ++++++++++++------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index adfcda24f30d..a47728386d21 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -8,7 +8,7 @@ export const NOTIFY_WITH_ALL_PARAMS = Symbol(); * @template TEntityInstance * @template {(keyof TEntityInstance)[]} TWriteProperties * @template {(keyof TEntityInstance)[]} TReadProperties - * @typedef {Partialvoid ,value: TEntityInstance, property: TWriteProperties[number], ...params: unknown[])=>boolean>>} Interceptors - return false if you want to prevent reactivity for this call, DO NOT USE INTERCEPTORS FOR READ METHODS + * @typedef {Partialvoid ,value: TEntityInstance, property: TWriteProperties[number], ...params: unknown[])=>boolean>>} Interceptors - return false if you want to prevent reactivity for this call, DO NOT USE INTERCEPTORS FOR READ PROPERTIES */ /** @@ -16,18 +16,18 @@ export const NOTIFY_WITH_ALL_PARAMS = Symbol(); * @template {(keyof TEntityInstance)[]} TWriteProperties * @template {(keyof TEntityInstance)[]} TReadProperties * @typedef {object} Options - * @prop {TWriteProperties} write_properties - an array of property names on `TEntityInstance` that when calling a property on `TEntityInstance`, if the property name exists in this array, then mentioned property causes reactivity. - * @prop {TReadProperties} read_properties - an array of property names on `TEntityInstance` that `mutation_properties` affects - * @prop {Interceptors} [interceptors={}] - if the property names in `mutation_properties` shouldn't cause reactivity, such calling `set.add(2)` twice or accessing a property shouldn't be reactive based on some conditions, you can prevent the reactivity by returning `false` from these interceptors + * @prop {TWriteProperties} write_properties - an array of property names on `TEntityInstance`, could cause reactivity. + * @prop {TReadProperties} read_properties - an array of property names on `TEntityInstance` that `write_properties` affect. typically used for methods. for instance `size` doesn't need to be here because it takes no parameters and is reactive based on the `version` signal. + * @prop {Interceptors} [interceptors={}] - if the property names in `write_properties` shouldn't cause reactivity, such as calling `set.add(2)` twice or accessing a property shouldn't be reactive based on some conditions, you can prevent the reactivity by returning `false` from these interceptors */ /** @typedef {Map>>} ReadMethodsSignals */ /** - * @template {new (...args: any) => any} TEntity - the entity we want to make reactive + * @template {new (...args: any) => any} TEntity * @template {(keyof InstanceType)[]} TWriteProperties * @template {(keyof InstanceType)[]} TReadProperties - * @param {TEntity} Entity - the class/function we want to make reactive + * @param {TEntity} Entity - the entity we want to make reactive * @param {Options, TWriteProperties, TReadProperties>} options - configurations for how reactivity works for this entity * @returns {TEntity} */ @@ -40,11 +40,9 @@ export const make_reactive = (Entity, options) => { */ constructor(...params) { /** - * each read method can be tracked like has, size, get and etc. these props might depend on a parameter. they have to reactive based on the - * parameter they depend on, if it requires a change. for instance if you call `set.has(2)` then call `set.add(5)` the former shouldn't get notified. - * so what is do we need to store if this needs to be generic? function name + parameter. unfortunately for each parameter we need a new signal, why? - * because arrays don't equal themselves even if they have the same value (referential equality). - * fortunately current builtins that we try to make reactive only take 1 parameter for their read methods so this is fine. + * each read method can be tracked like has, get, has and etc. these props might depend on a parameter. they have to reactive based on the + * parameter they depend on. for instance if you have `set.has(2)` and then call `set.add(5)` the former shouldn't get notified. + * based on that we need to store the function_name + parameter(s). * @type {Map>>} **/ const read_methods_signals = new Map(); @@ -58,7 +56,7 @@ export const make_reactive = (Entity, options) => { let result; if (typeof orig_property === 'function') { - // Bind functions directly to the `TEntity` + // bind functions directly to the `TEntity` result = ((/** @type {unknown[]} */ ...params) => { notify_if_required( version_signal, @@ -71,7 +69,7 @@ export const make_reactive = (Entity, options) => { return orig_property.bind(target)(...params); }).bind(target); } else { - // Properly handle getters + // handle getters/props notify_if_required(version_signal, read_methods_signals, property, target, options); result = Reflect.get(target, property, target); } @@ -127,7 +125,7 @@ function notify_if_required( } else { if (options.read_properties.includes(property)) { (params.length == 0 ? [null] : params).forEach((param) => { - // read like methods should create the signal so what on any change to them they get notified + // read like methods should create the signal (if not already created) so they can be reactive when notified based on their param const sig = get_signal_for_function(read_methods_signals, property, param, true); get(sig); }); @@ -146,7 +144,7 @@ function notify_if_required( * @param {ReadMethodsSignals} read_methods_signals * @param {Options, TWriteProperties, TReadProperties>} options * @param {TReadProperties} method_names - * @param {unknown[]} params - if you want to notify for all parameters pass NOTIFY_WITH_ALL_PARAMS constant, for instance some methods like `clear` should notify all `something.get(x)` methods; on these cases set this flag to true + * @param {unknown[]} params - if you want to notify for all parameters pass the `NOTIFY_WITH_ALL_PARAMS` constant, for instance some methods like `clear` should notify all `something.get(x)` methods; on these cases set this flag to true */ function notify_read_methods( options, @@ -175,7 +173,7 @@ function notify_read_methods( } /** - * gets the signal for this function based on params. If the signal doesn't exist and create_signal_if_doesnt_exist is not set to true, it creates a new one and returns that + * gets the signal for this function based on params. If the signal doesn't exist and `create_signal_if_doesnt_exist` is not set to true, it creates a new one and returns that * @template {boolean} [TCreateSignalIfDoesntExist=false] * @param {ReadMethodsSignals} signals_map * @param {string | symbol | number} function_name @@ -187,7 +185,7 @@ const get_signal_for_function = ( signals_map, function_name, param, - // @ts-ignore: this should be supported in jsdoc based on https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html#template but doesn't? + // @ts-ignore: this should be supported in jsdoc based on https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html#template but isn't? create_signal_if_doesnt_exist = false ) => { /** From 9715aa8509bec9139d324284c35b7f16a52927cd Mon Sep 17 00:00:00 2001 From: godzylinux Date: Sat, 18 May 2024 19:55:21 +0330 Subject: [PATCH 26/71] notifying other signals of changes, AFTER the change has actually took effect --- packages/svelte/src/reactivity/utils.js | 62 ++++++++++++++++--------- 1 file changed, 39 insertions(+), 23 deletions(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index a47728386d21..129bd4f418e5 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -58,7 +58,7 @@ export const make_reactive = (Entity, options) => { if (typeof orig_property === 'function') { // bind functions directly to the `TEntity` result = ((/** @type {unknown[]} */ ...params) => { - notify_if_required( + const notifiers = create_notifiers( version_signal, read_methods_signals, property, @@ -66,12 +66,21 @@ export const make_reactive = (Entity, options) => { options, ...params ); - return orig_property.bind(target)(...params); + const result = orig_property.bind(target)(...params); + notifiers.forEach((notifier) => notifier()); + return result; }).bind(target); } else { // handle getters/props - notify_if_required(version_signal, read_methods_signals, property, target, options); + const notifiers = create_notifiers( + version_signal, + read_methods_signals, + property, + target, + options + ); result = Reflect.get(target, property, target); + notifiers.forEach((notifier) => notifier()); } return result; @@ -82,6 +91,7 @@ export const make_reactive = (Entity, options) => { }; /** + * creates an array of functions that notify other signals based on the changes, you need to run these functions to invoke reactivity * @template {new (...args: any) => any} TEntity * @template {(keyof TEntityInstance)[]} TWriteProperties * @template {(keyof TEntityInstance)[]} TReadProperties @@ -93,8 +103,9 @@ export const make_reactive = (Entity, options) => { * @param {TEntityInstance} entity_instance * @param {Options, TWriteProperties, TReadProperties>} options * @param {unknown[]} params + * @returns {Function[]} */ -function notify_if_required( +function create_notifiers( version_signal, read_methods_signals, property, @@ -102,37 +113,42 @@ function notify_if_required( options, ...params ) { + /** + * @type {Function[]} + */ + const notifiers = []; if ( options.interceptors?.[property]?.( (methods, ...params) => { - return notify_read_methods( - options, - version_signal, - read_methods_signals, - methods, - ...params - ); + notifiers.push(() => { + notify_read_methods(options, version_signal, read_methods_signals, methods, ...params); + }); }, entity_instance, property, ...params ) === false ) { - return; + return notifiers; } - if (options.write_properties.some((v) => v === property)) { - increment_signal(version_signal); - } else { - if (options.read_properties.includes(property)) { - (params.length == 0 ? [null] : params).forEach((param) => { - // read like methods should create the signal (if not already created) so they can be reactive when notified based on their param - const sig = get_signal_for_function(read_methods_signals, property, param, true); - get(sig); - }); + + notifiers.push(() => { + if (options.write_properties.some((v) => v === property)) { + increment_signal(version_signal); } else { - get(version_signal); + if (options.read_properties.includes(property)) { + (params.length == 0 ? [null] : params).forEach((param) => { + // read like methods should create the signal (if not already created) so they can be reactive when notified based on their param + const sig = get_signal_for_function(read_methods_signals, property, param, true); + get(sig); + }); + } else { + get(version_signal); + } } - } + }); + + return notifiers; } /** From d8bc4c7a7f8add056dd00af241d6172ea3394aff Mon Sep 17 00:00:00 2001 From: godzylinux Date: Sat, 18 May 2024 20:22:29 +0330 Subject: [PATCH 27/71] added fix to work with $inspect rune --- packages/svelte/src/reactivity/utils.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 129bd4f418e5..0c4af7556ee9 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -84,6 +84,11 @@ export const make_reactive = (Entity, options) => { } return result; + }, + ownKeys: (target) => { + // to make it work with $inspect + get(version_signal); + return Reflect.ownKeys(target); } }); } From 58b8c10f39ce45a8b056504656389e81956edfda Mon Sep 17 00:00:00 2001 From: godzylinux Date: Sat, 18 May 2024 22:40:02 +0330 Subject: [PATCH 28/71] optimized version signal updates --- packages/svelte/src/reactivity/utils.js | 35 ++++++++++++++++++------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 0c4af7556ee9..4d0e100710c4 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -118,28 +118,39 @@ function create_notifiers( options, ...params ) { + // the `version_signal_incremented` flag helps to update the `version_signal` only once + const options_with_version_flag = { ...options, is_version_signal_incremented: false }; + /** * @type {Function[]} */ const notifiers = []; - if ( + + const is_property_reactive = options.interceptors?.[property]?.( (methods, ...params) => { notifiers.push(() => { - notify_read_methods(options, version_signal, read_methods_signals, methods, ...params); + notify_read_methods( + options_with_version_flag, + version_signal, + read_methods_signals, + methods, + ...params + ); }); }, entity_instance, property, ...params - ) === false - ) { + ) === true; + + if (!is_property_reactive) { return notifiers; } notifiers.push(() => { if (options.write_properties.some((v) => v === property)) { - increment_signal(version_signal); + increment_signal(options_with_version_flag, version_signal); } else { if (options.read_properties.includes(property)) { (params.length == 0 ? [null] : params).forEach((param) => { @@ -163,7 +174,7 @@ function create_notifiers( * @template {InstanceType} TEntityInstance * @param {import('#client').Source} version_signal * @param {ReadMethodsSignals} read_methods_signals - * @param {Options, TWriteProperties, TReadProperties>} options + * @param {Options, TWriteProperties, TReadProperties> & {is_version_signal_incremented: boolean}} options * @param {TReadProperties} method_names * @param {unknown[]} params - if you want to notify for all parameters pass the `NOTIFY_WITH_ALL_PARAMS` constant, for instance some methods like `clear` should notify all `something.get(x)` methods; on these cases set this flag to true */ @@ -182,12 +193,12 @@ function notify_read_methods( } if (params.length == 1 && params[0] == NOTIFY_WITH_ALL_PARAMS) { read_methods_signals.get(name)?.forEach((sig) => { - increment_signal(version_signal, sig); + increment_signal(options, version_signal, sig); }); } else { (params.length == 0 ? [null] : params).forEach((param) => { const sig = get_signal_for_function(read_methods_signals, name, param); - sig && increment_signal(version_signal, sig); + sig && increment_signal(options, version_signal, sig); }); } }); @@ -248,10 +259,14 @@ const get_signal_for_function = ( /** * toggles the signal value. this change notifies any reactions (not using number explicitly cause its not required, change from true to false or vice versa is enough). + * @param {{is_version_signal_incremented: boolean}} options - this prevents changing the version signal multiple times in a single changeset * @param {import("#client").Source} version_signal * @param {import("#client").Source} [read_method_signal] */ -const increment_signal = (version_signal, read_method_signal) => { - set(version_signal, !version_signal.v); +const increment_signal = (options, version_signal, read_method_signal) => { + if (!options.is_version_signal_incremented) { + options.is_version_signal_incremented = true; + set(version_signal, !version_signal.v); + } read_method_signal && set(read_method_signal, !read_method_signal.v); }; From 77405de103c753334f94d5f491a7798ca2b62fd5 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Sat, 18 May 2024 23:52:42 +0330 Subject: [PATCH 29/71] simplified reactivity for map --- packages/svelte/src/reactivity/map.js | 9 ++++----- packages/svelte/src/reactivity/utils.js | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/svelte/src/reactivity/map.js b/packages/svelte/src/reactivity/map.js index 7ed7d01df9aa..c660eee1d97d 100644 --- a/packages/svelte/src/reactivity/map.js +++ b/packages/svelte/src/reactivity/map.js @@ -2,7 +2,7 @@ import { make_reactive, NOTIFY_WITH_ALL_PARAMS } from './utils.js'; export const ReactiveMap = make_reactive(Map, { write_properties: ['clear', 'delete', 'set'], - read_properties: ['get', 'keys', 'entries', 'values', 'has'], + read_properties: ['get', 'keys', 'has'], interceptors: { set: (notify_read_methods, value, property, ...params) => { if (value.get(params[0]) === params[1]) { @@ -11,15 +11,14 @@ export const ReactiveMap = make_reactive(Map, { if (!value.has(params[0])) { notify_read_methods(['keys']); } - notify_read_methods(['entries', 'values']); - notify_read_methods(['get', 'has'], params[1]); + notify_read_methods(['get', 'has'], params[0]); return true; }, clear: (notify_read_methods, value, property, ...params) => { if (value.size === 0) { return false; } - notify_read_methods(['keys', 'values', 'entries', 'has'], NOTIFY_WITH_ALL_PARAMS); + notify_read_methods(['keys', 'has'], NOTIFY_WITH_ALL_PARAMS); return true; }, delete: (notify_read_methods, value, property, ...params) => { @@ -27,7 +26,7 @@ export const ReactiveMap = make_reactive(Map, { return false; } notify_read_methods(['get', 'has'], params[0]); - notify_read_methods(['keys', 'values', 'entries']); + notify_read_methods(['keys']); return true; } } diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 4d0e100710c4..d6d826612bd8 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -142,7 +142,7 @@ function create_notifiers( entity_instance, property, ...params - ) === true; + ) !== false; // not saying `===true` because not returning anything is considered true for this scenario as well. if (!is_property_reactive) { return notifiers; From 2d12cbab9102ac5509fdc2b286d87c8270836830 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Mon, 20 May 2024 18:44:42 +0330 Subject: [PATCH 30/71] improved comments and simplified code --- packages/svelte/src/reactivity/map.js | 2 +- packages/svelte/src/reactivity/utils.js | 54 +++++++++++++++---------- 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/packages/svelte/src/reactivity/map.js b/packages/svelte/src/reactivity/map.js index c660eee1d97d..1f1feb27a04e 100644 --- a/packages/svelte/src/reactivity/map.js +++ b/packages/svelte/src/reactivity/map.js @@ -18,7 +18,7 @@ export const ReactiveMap = make_reactive(Map, { if (value.size === 0) { return false; } - notify_read_methods(['keys', 'has'], NOTIFY_WITH_ALL_PARAMS); + notify_read_methods(['get', 'keys', 'has'], NOTIFY_WITH_ALL_PARAMS); return true; }, delete: (notify_read_methods, value, property, ...params) => { diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index d6d826612bd8..2c17a5db6f15 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -5,10 +5,17 @@ import { get } from '../internal/client/runtime.js'; export const NOTIFY_WITH_ALL_PARAMS = Symbol(); /** + * some `write_properties` require a custom logic to notify a change for read properties. + * for instance calling `set.add(2)` two times should not cause reactivity the second time. + * interceptor is called before the call is proxied to the actual object, so we can decide wether a change + * is actually going to happen or not. + * - if a `write_property` shouldn't increment the `version` signal return false from the interceptor. note that calling `notify_read_methods` WILL increase the `version` in all cases. + * returning false is only useful if do it before calling `notify_read_methods` like an if-guard that returns false early because no change has happened. + * - DO NOT USE INTERCEPTORS FOR READ PROPERTIES * @template TEntityInstance * @template {(keyof TEntityInstance)[]} TWriteProperties * @template {(keyof TEntityInstance)[]} TReadProperties - * @typedef {Partialvoid ,value: TEntityInstance, property: TWriteProperties[number], ...params: unknown[])=>boolean>>} Interceptors - return false if you want to prevent reactivity for this call, DO NOT USE INTERCEPTORS FOR READ PROPERTIES + * @typedef {Partialvoid ,value: TEntityInstance, property: TWriteProperties[number], ...params: unknown[])=>boolean>>} Interceptors */ /** @@ -17,8 +24,8 @@ export const NOTIFY_WITH_ALL_PARAMS = Symbol(); * @template {(keyof TEntityInstance)[]} TReadProperties * @typedef {object} Options * @prop {TWriteProperties} write_properties - an array of property names on `TEntityInstance`, could cause reactivity. - * @prop {TReadProperties} read_properties - an array of property names on `TEntityInstance` that `write_properties` affect. typically used for methods. for instance `size` doesn't need to be here because it takes no parameters and is reactive based on the `version` signal. - * @prop {Interceptors} [interceptors={}] - if the property names in `write_properties` shouldn't cause reactivity, such as calling `set.add(2)` twice or accessing a property shouldn't be reactive based on some conditions, you can prevent the reactivity by returning `false` from these interceptors + * @prop {TReadProperties} read_properties - an array of property names on `TEntityInstance` that `write_properties` affect, typically used for methods. for instance `size` doesn't need to be here because it takes no parameters and is reactive based on the `version` signal. + * @prop {Interceptors} [interceptors={}] - an object of interceptors for `write_properties` that can customize how/when a `read_properties` should be notified of a change. */ /** @typedef {Map>>} ReadMethodsSignals */ @@ -126,26 +133,29 @@ function create_notifiers( */ const notifiers = []; - const is_property_reactive = - options.interceptors?.[property]?.( - (methods, ...params) => { - notifiers.push(() => { - notify_read_methods( - options_with_version_flag, - version_signal, - read_methods_signals, - methods, - ...params - ); - }); - }, - entity_instance, - property, - ...params - ) !== false; // not saying `===true` because not returning anything is considered true for this scenario as well. + const interceptor = options.interceptors?.[property]; + if (interceptor) { + const increment_version_signal = + interceptor( + (methods, ...params) => { + notifiers.push(() => { + notify_read_methods( + options_with_version_flag, + version_signal, + read_methods_signals, + methods, + ...params + ); + }); + }, + entity_instance, + property, + ...params + ) === true; - if (!is_property_reactive) { - return notifiers; + if (!increment_version_signal) { + return notifiers; + } } notifiers.push(() => { From dc89f8de2de2dccc7febd205e991828fdfa775f7 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Mon, 20 May 2024 19:25:15 +0330 Subject: [PATCH 31/71] ran build which auto updated index.d.ts --- packages/svelte/types/index.d.ts | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 48827bfc7e00..20b10162b03b 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -2079,20 +2079,8 @@ declare module 'svelte/reactivity' { constructor(...values: any[]); #private; } - class ReactiveSet extends Set { - - constructor(value?: Iterable | null | undefined); - - add(value: T): this; - #private; - } - class ReactiveMap extends Map { - - constructor(value?: Iterable | null | undefined); - - set(key: K, value: V): this; - #private; - } + export const Set: SetConstructor; + export const Map: MapConstructor; class ReactiveURL extends URL { get searchParams(): ReactiveURLSearchParams; #private; @@ -2104,7 +2092,7 @@ declare module 'svelte/reactivity' { } const REPLACE: unique symbol; - export { ReactiveDate as Date, ReactiveSet as Set, ReactiveMap as Map, ReactiveURL as URL, ReactiveURLSearchParams as URLSearchParams }; + export { ReactiveDate as Date, ReactiveURL as URL, ReactiveURLSearchParams as URLSearchParams }; } declare module 'svelte/server' { From 38a0d5e53bf54698e0b46d05ac5ffcb1c63159dc Mon Sep 17 00:00:00 2001 From: godzylinux Date: Mon, 20 May 2024 19:50:09 +0330 Subject: [PATCH 32/71] fixed linter issue --- packages/svelte/src/reactivity/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 2c17a5db6f15..796fa7fac77c 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -43,7 +43,7 @@ export const make_reactive = (Entity, options) => { // @ts-ignore return class { /** - * @param {ConstructorParameters} params + * @param {...unknown[]} params */ constructor(...params) { /** From 662a48d0e8e0e421b5e517acf348598879d6d467 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Mon, 20 May 2024 20:03:27 +0330 Subject: [PATCH 33/71] made read_properties optional --- packages/svelte/src/reactivity/utils.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 796fa7fac77c..21e4dde89b7b 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -24,7 +24,7 @@ export const NOTIFY_WITH_ALL_PARAMS = Symbol(); * @template {(keyof TEntityInstance)[]} TReadProperties * @typedef {object} Options * @prop {TWriteProperties} write_properties - an array of property names on `TEntityInstance`, could cause reactivity. - * @prop {TReadProperties} read_properties - an array of property names on `TEntityInstance` that `write_properties` affect, typically used for methods. for instance `size` doesn't need to be here because it takes no parameters and is reactive based on the `version` signal. + * @prop {TReadProperties} [read_properties] - an array of property names on `TEntityInstance` that `write_properties` affect, typically used for methods. for instance `size` doesn't need to be here because it takes no parameters and is reactive based on the `version` signal. * @prop {Interceptors} [interceptors={}] - an object of interceptors for `write_properties` that can customize how/when a `read_properties` should be notified of a change. */ @@ -162,7 +162,7 @@ function create_notifiers( if (options.write_properties.some((v) => v === property)) { increment_signal(options_with_version_flag, version_signal); } else { - if (options.read_properties.includes(property)) { + if (options.read_properties?.includes(property)) { (params.length == 0 ? [null] : params).forEach((param) => { // read like methods should create the signal (if not already created) so they can be reactive when notified based on their param const sig = get_signal_for_function(read_methods_signals, property, param, true); @@ -196,7 +196,7 @@ function notify_read_methods( ...params ) { method_names.forEach((name) => { - if (DEV && !options.read_properties.includes(name)) { + if (DEV && !options.read_properties?.includes(name)) { throw new Error( `when trying to notify reactions got a read method that wasn't defined in options: ${name.toString()}` ); From cfc219e0d31f308a1a0407f335c9348ddbea3441 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Tue, 21 May 2024 00:59:48 +0330 Subject: [PATCH 34/71] simplify and improve types --- packages/svelte/src/reactivity/utils.js | 84 ++++++++++++++++--------- 1 file changed, 56 insertions(+), 28 deletions(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 21e4dde89b7b..2db1e009851d 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -14,14 +14,14 @@ export const NOTIFY_WITH_ALL_PARAMS = Symbol(); * - DO NOT USE INTERCEPTORS FOR READ PROPERTIES * @template TEntityInstance * @template {(keyof TEntityInstance)[]} TWriteProperties - * @template {(keyof TEntityInstance)[]} TReadProperties + * @template {(keyof TEntityInstance)[] | undefined} TReadProperties * @typedef {Partialvoid ,value: TEntityInstance, property: TWriteProperties[number], ...params: unknown[])=>boolean>>} Interceptors */ /** * @template TEntityInstance * @template {(keyof TEntityInstance)[]} TWriteProperties - * @template {(keyof TEntityInstance)[]} TReadProperties + * @template {(keyof TEntityInstance)[] | undefined} TReadProperties * @typedef {object} Options * @prop {TWriteProperties} write_properties - an array of property names on `TEntityInstance`, could cause reactivity. * @prop {TReadProperties} [read_properties] - an array of property names on `TEntityInstance` that `write_properties` affect, typically used for methods. for instance `size` doesn't need to be here because it takes no parameters and is reactive based on the `version` signal. @@ -33,7 +33,7 @@ export const NOTIFY_WITH_ALL_PARAMS = Symbol(); /** * @template {new (...args: any) => any} TEntity * @template {(keyof InstanceType)[]} TWriteProperties - * @template {(keyof InstanceType)[]} TReadProperties + * @template {(keyof InstanceType)[] | undefined} TReadProperties * @param {TEntity} Entity - the entity we want to make reactive * @param {Options, TWriteProperties, TReadProperties>} options - configurations for how reactivity works for this entity * @returns {TEntity} @@ -50,7 +50,7 @@ export const make_reactive = (Entity, options) => { * each read method can be tracked like has, get, has and etc. these props might depend on a parameter. they have to reactive based on the * parameter they depend on. for instance if you have `set.has(2)` and then call `set.add(5)` the former shouldn't get notified. * based on that we need to store the function_name + parameter(s). - * @type {Map>>} + * @type {ReadMethodsSignals} **/ const read_methods_signals = new Map(); /** @@ -73,25 +73,33 @@ export const make_reactive = (Entity, options) => { options, ...params ); - const result = orig_property.bind(target)(...params); + const function_result = orig_property.bind(target)(...params); + // causing reactivity after the function is actually called and performed its changes + get_read_signals(version_signal, read_methods_signals, property, options, ...params); notifiers.forEach((notifier) => notifier()); - return result; + return function_result; }).bind(target); } else { // handle getters/props - const notifiers = create_notifiers( - version_signal, - read_methods_signals, - property, - target, - options - ); result = Reflect.get(target, property, target); - notifiers.forEach((notifier) => notifier()); + get_read_signals(version_signal, read_methods_signals, property, options, ...params); } return result; }, + set(target, property, value) { + const notifiers = create_notifiers( + version_signal, + read_methods_signals, + property, + target, + options, + value + ); + const result = Reflect.set(target, property, value, target); + notifiers.forEach((notifier) => notifier()); + return result; + }, ownKeys: (target) => { // to make it work with $inspect get(version_signal); @@ -106,7 +114,7 @@ export const make_reactive = (Entity, options) => { * creates an array of functions that notify other signals based on the changes, you need to run these functions to invoke reactivity * @template {new (...args: any) => any} TEntity * @template {(keyof TEntityInstance)[]} TWriteProperties - * @template {(keyof TEntityInstance)[]} TReadProperties + * @template {(keyof TEntityInstance)[] | undefined} TReadProperties * @template {InstanceType} TEntityInstance * @template {keyof TEntityInstance} TProperty * @param {import('#client').Source} version_signal @@ -133,7 +141,11 @@ function create_notifiers( */ const notifiers = []; - const interceptor = options.interceptors?.[property]; + const interceptor = + options.interceptors && + Object.hasOwn(options.interceptors, property) && + options.interceptors?.[property]; + if (interceptor) { const increment_version_signal = interceptor( @@ -161,16 +173,6 @@ function create_notifiers( notifiers.push(() => { if (options.write_properties.some((v) => v === property)) { increment_signal(options_with_version_flag, version_signal); - } else { - if (options.read_properties?.includes(property)) { - (params.length == 0 ? [null] : params).forEach((param) => { - // read like methods should create the signal (if not already created) so they can be reactive when notified based on their param - const sig = get_signal_for_function(read_methods_signals, property, param, true); - get(sig); - }); - } else { - get(version_signal); - } } }); @@ -180,7 +182,33 @@ function create_notifiers( /** * @template {new (...args: any) => any} TEntity * @template {(keyof TEntityInstance)[]} TWriteProperties - * @template {(keyof TEntityInstance)[]} TReadProperties + * @template {(keyof TEntityInstance)[] | undefined} TReadProperties + * @template {InstanceType} TEntityInstance + * @template {keyof TEntityInstance} TProperty + * @param {import('#client').Source} version_signal + * @param {ReadMethodsSignals} read_methods_signals + * @param {TProperty} property + * @param {Options, TWriteProperties, TReadProperties>} options + * @param {unknown[]} params + */ + +function get_read_signals(version_signal, read_methods_signals, property, options, ...params) { + if (options.read_properties?.includes(property)) { + (params.length == 0 ? [null] : params).forEach((param) => { + // read like methods that are reactive conditionally should create the signal (if not already created) so they can be reactive when notified based on their param + const sig = get_signal_for_function(read_methods_signals, property, param, true); + get(sig); + }); + } else { + // other read like methods that are not reactive conditionally based their params and are just notified based on the version signal are here + get(version_signal); + } +} + +/** + * @template {new (...args: any) => any} TEntity + * @template {(keyof TEntityInstance)[]} TWriteProperties + * @template {(keyof TEntityInstance)[] | undefined} TReadProperties * @template {InstanceType} TEntityInstance * @param {import('#client').Source} version_signal * @param {ReadMethodsSignals} read_methods_signals @@ -195,7 +223,7 @@ function notify_read_methods( method_names, ...params ) { - method_names.forEach((name) => { + method_names?.forEach((name) => { if (DEV && !options.read_properties?.includes(name)) { throw new Error( `when trying to notify reactions got a read method that wasn't defined in options: ${name.toString()}` From 90531c1557959391d4d5cf899b0d529b5af8beca Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 22 May 2024 11:14:36 +0330 Subject: [PATCH 35/71] improved utility for reactivity package so if we wanted to we can export stuff from the base proxied class --- packages/svelte/src/reactivity/utils.js | 86 +++++++++++++++---------- 1 file changed, 51 insertions(+), 35 deletions(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 2db1e009851d..de6931deaec2 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -14,18 +14,18 @@ export const NOTIFY_WITH_ALL_PARAMS = Symbol(); * - DO NOT USE INTERCEPTORS FOR READ PROPERTIES * @template TEntityInstance * @template {(keyof TEntityInstance)[]} TWriteProperties - * @template {(keyof TEntityInstance)[] | undefined} TReadProperties + * @template {(keyof TEntityInstance)[]} TReadProperties * @typedef {Partialvoid ,value: TEntityInstance, property: TWriteProperties[number], ...params: unknown[])=>boolean>>} Interceptors */ /** * @template TEntityInstance * @template {(keyof TEntityInstance)[]} TWriteProperties - * @template {(keyof TEntityInstance)[] | undefined} TReadProperties + * @template {(keyof TEntityInstance)[]} TReadProperties * @typedef {object} Options * @prop {TWriteProperties} write_properties - an array of property names on `TEntityInstance`, could cause reactivity. * @prop {TReadProperties} [read_properties] - an array of property names on `TEntityInstance` that `write_properties` affect, typically used for methods. for instance `size` doesn't need to be here because it takes no parameters and is reactive based on the `version` signal. - * @prop {Interceptors} [interceptors={}] - an object of interceptors for `write_properties` that can customize how/when a `read_properties` should be notified of a change. + * @prop {Interceptors} [interceptors] - an object of interceptors for `write_properties` that can customize how/when a `read_properties` should be notified of a change. */ /** @typedef {Map>>} ReadMethodsSignals */ @@ -33,7 +33,7 @@ export const NOTIFY_WITH_ALL_PARAMS = Symbol(); /** * @template {new (...args: any) => any} TEntity * @template {(keyof InstanceType)[]} TWriteProperties - * @template {(keyof InstanceType)[] | undefined} TReadProperties + * @template {(keyof InstanceType)[]} TReadProperties * @param {TEntity} Entity - the entity we want to make reactive * @param {Options, TWriteProperties, TReadProperties>} options - configurations for how reactivity works for this entity * @returns {TEntity} @@ -41,68 +41,84 @@ export const NOTIFY_WITH_ALL_PARAMS = Symbol(); export const make_reactive = (Entity, options) => { // we return a class so that the caller can call it with new // @ts-ignore - return class { + return class extends Entity { + /** + * each read method can be tracked like has, get, has and etc. these props might depend on a parameter. they have to reactive based on the + * parameter they depend on. for instance if you have `set.has(2)` and then call `set.add(5)` the former shouldn't get notified. + * based on that we need to store the function_name + parameter(s). + * @type {ReadMethodsSignals} + **/ + #read_methods_signals = new Map(); + + /** + * other props that get notified based on any change listen to version + */ + #version_signal = source(false); + /** * @param {...unknown[]} params */ + constructor(...params) { - /** - * each read method can be tracked like has, get, has and etc. these props might depend on a parameter. they have to reactive based on the - * parameter they depend on. for instance if you have `set.has(2)` and then call `set.add(5)` the former shouldn't get notified. - * based on that we need to store the function_name + parameter(s). - * @type {ReadMethodsSignals} - **/ - const read_methods_signals = new Map(); - /** - * other props that get notified based on any change listen to version - */ - const version_signal = source(false); - return new Proxy(new Entity(...params), { - get(target, property) { - const orig_property = target[property]; + super(...params); + return new Proxy(this, { + get: (target, property) => { + const orig_property = target[/**@type {keyof typeof target}*/ (property)]; let result; if (typeof orig_property === 'function') { // bind functions directly to the `TEntity` result = ((/** @type {unknown[]} */ ...params) => { const notifiers = create_notifiers( - version_signal, - read_methods_signals, + target.#version_signal, + target.#read_methods_signals, property, - target, + /**@type {InstanceType}*/ (target), options, ...params ); const function_result = orig_property.bind(target)(...params); // causing reactivity after the function is actually called and performed its changes - get_read_signals(version_signal, read_methods_signals, property, options, ...params); + get_read_signals( + target.#version_signal, + target.#read_methods_signals, + property, + options, + ...params + ); notifiers.forEach((notifier) => notifier()); return function_result; }).bind(target); } else { // handle getters/props - result = Reflect.get(target, property, target); - get_read_signals(version_signal, read_methods_signals, property, options, ...params); + result = Reflect.get(target, property); + get_read_signals( + target.#version_signal, + target.#read_methods_signals, + property, + options, + ...params + ); } return result; }, set(target, property, value) { const notifiers = create_notifiers( - version_signal, - read_methods_signals, + target.#version_signal, + target.#read_methods_signals, property, - target, + /**@type {InstanceType}*/ (target), options, value ); - const result = Reflect.set(target, property, value, target); + const result = Reflect.set(target, property, value); notifiers.forEach((notifier) => notifier()); return result; }, ownKeys: (target) => { // to make it work with $inspect - get(version_signal); + get(target.#version_signal); return Reflect.ownKeys(target); } }); @@ -114,7 +130,7 @@ export const make_reactive = (Entity, options) => { * creates an array of functions that notify other signals based on the changes, you need to run these functions to invoke reactivity * @template {new (...args: any) => any} TEntity * @template {(keyof TEntityInstance)[]} TWriteProperties - * @template {(keyof TEntityInstance)[] | undefined} TReadProperties + * @template {(keyof TEntityInstance)[]} TReadProperties * @template {InstanceType} TEntityInstance * @template {keyof TEntityInstance} TProperty * @param {import('#client').Source} version_signal @@ -182,7 +198,7 @@ function create_notifiers( /** * @template {new (...args: any) => any} TEntity * @template {(keyof TEntityInstance)[]} TWriteProperties - * @template {(keyof TEntityInstance)[] | undefined} TReadProperties + * @template {(keyof TEntityInstance)[]} TReadProperties * @template {InstanceType} TEntityInstance * @template {keyof TEntityInstance} TProperty * @param {import('#client').Source} version_signal @@ -199,7 +215,7 @@ function get_read_signals(version_signal, read_methods_signals, property, option const sig = get_signal_for_function(read_methods_signals, property, param, true); get(sig); }); - } else { + } else if (!options.write_properties.includes(property)) { // other read like methods that are not reactive conditionally based their params and are just notified based on the version signal are here get(version_signal); } @@ -208,12 +224,12 @@ function get_read_signals(version_signal, read_methods_signals, property, option /** * @template {new (...args: any) => any} TEntity * @template {(keyof TEntityInstance)[]} TWriteProperties - * @template {(keyof TEntityInstance)[] | undefined} TReadProperties + * @template {(keyof TEntityInstance)[]} TReadProperties * @template {InstanceType} TEntityInstance * @param {import('#client').Source} version_signal * @param {ReadMethodsSignals} read_methods_signals * @param {Options, TWriteProperties, TReadProperties> & {is_version_signal_incremented: boolean}} options - * @param {TReadProperties} method_names + * @param {TReadProperties | undefined} method_names * @param {unknown[]} params - if you want to notify for all parameters pass the `NOTIFY_WITH_ALL_PARAMS` constant, for instance some methods like `clear` should notify all `something.get(x)` methods; on these cases set this flag to true */ function notify_read_methods( From 42bf1e995fbe04f936974b79534d3c4244fe9fdb Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 22 May 2024 11:15:46 +0330 Subject: [PATCH 36/71] changed reactivity/url to use the new utility, also made it fine-grained reactive --- .../svelte/src/reactivity/index-client.js | 3 +- .../src/reactivity/url-reach-params.test.ts | 27 ++ .../src/reactivity/url-search-params.js | 29 ++ packages/svelte/src/reactivity/url.js | 368 +++++++----------- packages/svelte/src/reactivity/url.test.ts | 25 +- packages/svelte/types/index.d.ts | 14 +- 6 files changed, 204 insertions(+), 262 deletions(-) create mode 100644 packages/svelte/src/reactivity/url-reach-params.test.ts create mode 100644 packages/svelte/src/reactivity/url-search-params.js diff --git a/packages/svelte/src/reactivity/index-client.js b/packages/svelte/src/reactivity/index-client.js index 9c0b28dd2f34..2beacca942c6 100644 --- a/packages/svelte/src/reactivity/index-client.js +++ b/packages/svelte/src/reactivity/index-client.js @@ -1,4 +1,5 @@ export { ReactiveDate as Date } from './date.js'; export { ReactiveSet as Set } from './set.js'; export { ReactiveMap as Map } from './map.js'; -export { ReactiveURL as URL, ReactiveURLSearchParams as URLSearchParams } from './url.js'; +export { ReactiveURL as URL } from './url.js'; +export { ReactiveURLSearchParams as URLSearchParams } from './url-search-params.js'; diff --git a/packages/svelte/src/reactivity/url-reach-params.test.ts b/packages/svelte/src/reactivity/url-reach-params.test.ts new file mode 100644 index 000000000000..d6937004676b --- /dev/null +++ b/packages/svelte/src/reactivity/url-reach-params.test.ts @@ -0,0 +1,27 @@ +import { render_effect, effect_root } from '../internal/client/reactivity/effects.js'; +import { flushSync } from '../index-client.js'; +import { ReactiveURLSearchParams } from './url-search-params.js'; +import { assert, test } from 'vitest'; + +test('URLSearchParams', () => { + const params = new ReactiveURLSearchParams(); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(params.toString()); + }); + }); + + flushSync(() => { + params.set('a', 'b'); + }); + + flushSync(() => { + params.append('a', 'c'); + }); + + assert.deepEqual(log, ['', 'a=b', 'a=b&a=c']); + + cleanup(); +}); diff --git a/packages/svelte/src/reactivity/url-search-params.js b/packages/svelte/src/reactivity/url-search-params.js new file mode 100644 index 000000000000..8e7260dddddb --- /dev/null +++ b/packages/svelte/src/reactivity/url-search-params.js @@ -0,0 +1,29 @@ +import { make_reactive } from './utils.js'; + +export const ReactiveURLSearchParams = make_reactive(URLSearchParams, { + write_properties: ['append', 'delete', 'set', 'sort'], + read_properties: ['get', 'has'], + interceptors: { + set: (notify_read_methods, value, property, ...params) => { + if (typeof params[0] == 'string' && value.get(params[0]) === params[1]) { + return false; + } + notify_read_methods(['get', 'has'], params[0]); + return true; + }, + append: (notify_read_methods, value, property, ...params) => { + if (typeof params[0] == 'string' && value.get(params[0]) === params[1]) { + return false; + } + notify_read_methods(['get', 'has'], params[0]); + return true; + }, + delete: (notify_read_methods, value, property, ...params) => { + if (typeof params[0] == 'string' && !value.has(params[0])) { + return false; + } + notify_read_methods(['get', 'has'], params[0]); + return true; + } + } +}); diff --git a/packages/svelte/src/reactivity/url.js b/packages/svelte/src/reactivity/url.js index 3f11a4af37c7..1f5e8facf39c 100644 --- a/packages/svelte/src/reactivity/url.js +++ b/packages/svelte/src/reactivity/url.js @@ -1,268 +1,178 @@ -import { source, set } from '../internal/client/reactivity/sources.js'; -import { get } from '../internal/client/runtime.js'; - -const REPLACE = Symbol(); - -export class ReactiveURL extends URL { - #protocol = source(super.protocol); - #username = source(super.username); - #password = source(super.password); - #hostname = source(super.hostname); - #port = source(super.port); - #pathname = source(super.pathname); - #hash = source(super.hash); - #searchParams = new ReactiveURLSearchParams(); +import { ReactiveURLSearchParams } from './url-search-params.js'; +import { make_reactive } from './utils.js'; +class URLWithReactiveSearchParams extends URL { /** - * @param {string | URL} url - * @param {string | URL} [base] + * @type {InstanceType} */ - constructor(url, base) { - url = new URL(url, base); - super(url); - this.#searchParams[REPLACE](url.searchParams); - } - - get hash() { - return get(this.#hash); - } - - set hash(value) { - super.hash = value; - set(this.#hash, super.hash); - } - - get host() { - get(this.#hostname); - get(this.#port); - return super.host; - } - - set host(value) { - super.host = value; - set(this.#hostname, super.hostname); - set(this.#port, super.port); - } - - get hostname() { - return get(this.#hostname); - } - - set hostname(value) { - super.hostname = value; - set(this.#hostname, super.hostname); - } - - get href() { - get(this.#protocol); - get(this.#username); - get(this.#password); - get(this.#hostname); - get(this.#port); - get(this.#pathname); - get(this.#hash); - this.#searchParams.toString(); - return super.href; - } - - set href(value) { - super.href = value; - set(this.#protocol, super.protocol); - set(this.#username, super.username); - set(this.#password, super.password); - set(this.#hostname, super.hostname); - set(this.#port, super.port); - set(this.#pathname, super.pathname); - set(this.#hash, super.hash); - this.#searchParams[REPLACE](super.searchParams); - } - - get password() { - return get(this.#password); - } - - set password(value) { - super.password = value; - set(this.#password, super.password); - } - - get pathname() { - return get(this.#pathname); - } - - set pathname(value) { - super.pathname = value; - set(this.#pathname, super.pathname); - } - - get port() { - return get(this.#port); - } - - set port(value) { - super.port = value; - set(this.#port, super.port); - } - - get protocol() { - return get(this.#protocol); - } - - set protocol(value) { - super.protocol = value; - set(this.#protocol, super.protocol); - } - - get search() { - const search = this.#searchParams?.toString(); - return search ? `?${search}` : ''; - } - - set search(value) { - super.search = value; - this.#searchParams[REPLACE](new URLSearchParams(value.replace(/^\?/, ''))); - } - - get username() { - return get(this.#username); - } - - set username(value) { - super.username = value; - set(this.#username, super.username); - } - - get origin() { - get(this.#protocol); - get(this.#hostname); - get(this.#port); - return super.origin; - } - - get searchParams() { - return this.#searchParams; - } - - toString() { - return this.href; - } - - toJSON() { - return this.href; - } -} - -export class ReactiveURLSearchParams extends URLSearchParams { - #version = source(0); - - #increment_version() { - set(this.#version, this.#version.v + 1); - } + #reactive_search_params; /** - * @param {URLSearchParams} params + * @param {ConstructorParameters} params */ - [REPLACE](params) { - for (const key of [...super.keys()]) { - super.delete(key); - } - - for (const [key, value] of params) { - super.append(key, value); - } + constructor(...params) { + super(...params); - this.#increment_version(); + this.#reactive_search_params = new ReactiveURLSearchParams(super.searchParams); } /** - * @param {string} name - * @param {string} value - * @returns {void} + * @override */ - append(name, value) { - this.#increment_version(); - return super.append(name, value); + get searchParams() { + return this.#reactive_search_params; } /** - * @param {string} name - * @param {string=} value - * @returns {void} + * @override */ - delete(name, value) { - this.#increment_version(); - return super.delete(name, value); + get search() { + this.searchParams.toString(); + this.#sync_params_with_url({ search_params_updated: true }); + return super.search; } /** - * @param {string} name - * @returns {string|null} + * @override */ - get(name) { - get(this.#version); - return super.get(name); + set search(value) { + super.search = value; + this.#sync_params_with_url({ url_updated: true }); } /** - * @param {string} name - * @returns {string[]} + * @override */ - getAll(name) { - get(this.#version); - return super.getAll(name); + get href() { + this.searchParams.toString(); + this.#sync_params_with_url({ search_params_updated: true }); + return super.href; } /** - * @param {string} name - * @param {string=} value - * @returns {boolean} + * @override */ - has(name, value) { - get(this.#version); - return super.has(name, value); - } - - keys() { - get(this.#version); - return super.keys(); + set href(value) { + super.href = value; + this.#sync_params_with_url({ url_updated: true }); } /** - * @param {string} name - * @param {string} value - * @returns {void} + * @param {{url_updated?: boolean, search_params_updated?: boolean}} param0 */ - set(name, value) { - this.#increment_version(); - return super.set(name, value); - } + #sync_params_with_url({ url_updated = false, search_params_updated = false }) { + if (super.searchParams.toString() === this.searchParams.toString()) { + return; + } - sort() { - this.#increment_version(); - return super.sort(); + if (url_updated) { + Array.from(this.searchParams.keys()).forEach((key) => { + // remove keys that don't exist anymore + if (!super.searchParams.has(key)) { + this.searchParams.delete(key); + } + }); + + for (const key of super.searchParams.keys()) { + // set/update keys + this.searchParams.set(key, /** @type {string} */ (super.searchParams.get(key))); + } + } else if (search_params_updated) { + this.search = this.searchParams.toString(); + } } + /** + * @override + */ toString() { - get(this.#version); + this.searchParams.toString(); + this.#sync_params_with_url({ search_params_updated: true }); return super.toString(); } +} - values() { - get(this.#version); - return super.values(); - } - - entries() { - get(this.#version); - return super.entries(); - } - - [Symbol.iterator]() { - return this.entries(); - } - - get size() { - get(this.#version); - return super.size; +export const ReactiveURL = make_reactive(URLWithReactiveSearchParams, { + write_properties: [ + 'protocol', + 'username', + 'password', + 'hostname', + 'port', + 'pathname', + 'hash', + 'search', + 'href' + ], + read_properties: ['href', 'origin', 'host', 'hash', 'pathname'], + interceptors: { + protocol: (notify_read_methods, value, property, ...params) => { + if ( + typeof params[0] == 'string' && + value.protocol.split(':')[0] === params[0].split(':')[0] + ) { + return false; + } + notify_read_methods(['href', 'origin']); + return true; + }, + username: (notify_read_methods, value, property, ...params) => { + if (value.protocol === params[0]) { + return false; + } + notify_read_methods(['href']); + return true; + }, + password: (notify_read_methods, value, property, ...params) => { + if (value.protocol === params[0]) { + return false; + } + notify_read_methods(['href']); + return true; + }, + hostname: (notify_read_methods, value, property, ...params) => { + if (value.protocol === params[0]) { + return false; + } + notify_read_methods(['href', 'host']); + return true; + }, + + port: (notify_read_methods, value, property, ...params) => { + if (value.protocol === params[0]) { + return false; + } + notify_read_methods(['href', 'origin', 'host']); + return true; + }, + + pathname: (notify_read_methods, value, property, ...params) => { + if (value.protocol === params[0]) { + return false; + } + notify_read_methods(['href']); + return true; + }, + hash: (notify_read_methods, value, property, ...params) => { + if (value.protocol === params[0]) { + return false; + } + notify_read_methods(['href']); + return true; + }, + search: (notify_read_methods, value, property, ...params) => { + if (value.search === params[0]) { + return false; + } + notify_read_methods(['href', 'hash']); + return true; + }, + href: (notify_read_methods, value, property, ...params) => { + if (value.href === params[0]) { + return false; + } + notify_read_methods(['origin', 'host', 'hash', 'pathname']); + return true; + } } -} +}); diff --git a/packages/svelte/src/reactivity/url.test.ts b/packages/svelte/src/reactivity/url.test.ts index 910edba5021d..53f7796bc2ee 100644 --- a/packages/svelte/src/reactivity/url.test.ts +++ b/packages/svelte/src/reactivity/url.test.ts @@ -1,6 +1,6 @@ import { render_effect, effect_root } from '../internal/client/reactivity/effects.js'; import { flushSync } from '../index-client.js'; -import { ReactiveURL, ReactiveURLSearchParams } from './url.js'; +import { ReactiveURL } from './url.js'; import { assert, test } from 'vitest'; test('url.hash', () => { @@ -76,26 +76,3 @@ test('url.searchParams', () => { cleanup(); }); - -test('URLSearchParams', () => { - const params = new ReactiveURLSearchParams(); - const log: any = []; - - const cleanup = effect_root(() => { - render_effect(() => { - log.push(params.toString()); - }); - }); - - flushSync(() => { - params.set('a', 'b'); - }); - - flushSync(() => { - params.append('a', 'c'); - }); - - assert.deepEqual(log, ['', 'a=b', 'a=b&a=c']); - - cleanup(); -}); diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 20b10162b03b..9dbb2492e0fa 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -2081,16 +2081,14 @@ declare module 'svelte/reactivity' { } export const Set: SetConstructor; export const Map: MapConstructor; - class ReactiveURL extends URL { - get searchParams(): ReactiveURLSearchParams; + const ReactiveURL: typeof URLWithReactiveSearchParams; + class URLWithReactiveSearchParams extends URL { #private; } - class ReactiveURLSearchParams extends URLSearchParams { - - [REPLACE](params: URLSearchParams): void; - #private; - } - const REPLACE: unique symbol; + const ReactiveURLSearchParams: { + new (init?: string | Record | URLSearchParams | string[][] | undefined): URLSearchParams; + prototype: URLSearchParams; + }; export { ReactiveDate as Date, ReactiveURL as URL, ReactiveURLSearchParams as URLSearchParams }; } From 7ac79e6b6c453af42a9427185f08ad137124950e Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 22 May 2024 11:27:28 +0330 Subject: [PATCH 37/71] renamed notify_read_methods to notify_read_properties --- packages/svelte/src/reactivity/map.js | 16 ++++---- packages/svelte/src/reactivity/set.js | 12 +++--- .../src/reactivity/url-search-params.js | 12 +++--- packages/svelte/src/reactivity/url.js | 40 ++++++++++--------- packages/svelte/src/reactivity/utils.js | 10 ++--- 5 files changed, 47 insertions(+), 43 deletions(-) diff --git a/packages/svelte/src/reactivity/map.js b/packages/svelte/src/reactivity/map.js index 1f1feb27a04e..b1a6dac23abc 100644 --- a/packages/svelte/src/reactivity/map.js +++ b/packages/svelte/src/reactivity/map.js @@ -4,29 +4,29 @@ export const ReactiveMap = make_reactive(Map, { write_properties: ['clear', 'delete', 'set'], read_properties: ['get', 'keys', 'has'], interceptors: { - set: (notify_read_methods, value, property, ...params) => { + set: (notify_read_properties, value, property, ...params) => { if (value.get(params[0]) === params[1]) { return false; } if (!value.has(params[0])) { - notify_read_methods(['keys']); + notify_read_properties(['keys']); } - notify_read_methods(['get', 'has'], params[0]); + notify_read_properties(['get', 'has'], params[0]); return true; }, - clear: (notify_read_methods, value, property, ...params) => { + clear: (notify_read_properties, value, property, ...params) => { if (value.size === 0) { return false; } - notify_read_methods(['get', 'keys', 'has'], NOTIFY_WITH_ALL_PARAMS); + notify_read_properties(['get', 'keys', 'has'], NOTIFY_WITH_ALL_PARAMS); return true; }, - delete: (notify_read_methods, value, property, ...params) => { + delete: (notify_read_properties, value, property, ...params) => { if (!value.has(params[0])) { return false; } - notify_read_methods(['get', 'has'], params[0]); - notify_read_methods(['keys']); + notify_read_properties(['get', 'has'], params[0]); + notify_read_properties(['keys']); return true; } } diff --git a/packages/svelte/src/reactivity/set.js b/packages/svelte/src/reactivity/set.js index 52fc70decb0f..e7e69b4a1bbc 100644 --- a/packages/svelte/src/reactivity/set.js +++ b/packages/svelte/src/reactivity/set.js @@ -4,25 +4,25 @@ export const ReactiveSet = make_reactive(Set, { write_properties: ['add', 'clear', 'delete'], read_properties: ['has'], interceptors: { - add: (notify_read_methods, value, property, ...params) => { + add: (notify_read_properties, value, property, ...params) => { if (value.has(params[0])) { return false; } - notify_read_methods(['has'], params[0]); + notify_read_properties(['has'], params[0]); return true; }, - clear: (notify_read_methods, value, property, ...params) => { + clear: (notify_read_properties, value, property, ...params) => { if (value.size == 0) { return false; } - notify_read_methods(['has'], NOTIFY_WITH_ALL_PARAMS); + notify_read_properties(['has'], NOTIFY_WITH_ALL_PARAMS); return true; }, - delete: (notify_read_methods, value, property, ...params) => { + delete: (notify_read_properties, value, property, ...params) => { if (!value.has(params[0])) { return false; } - notify_read_methods(['has'], params[0]); + notify_read_properties(['has'], params[0]); return true; } } diff --git a/packages/svelte/src/reactivity/url-search-params.js b/packages/svelte/src/reactivity/url-search-params.js index 8e7260dddddb..0a55c0b249bd 100644 --- a/packages/svelte/src/reactivity/url-search-params.js +++ b/packages/svelte/src/reactivity/url-search-params.js @@ -4,25 +4,25 @@ export const ReactiveURLSearchParams = make_reactive(URLSearchParams, { write_properties: ['append', 'delete', 'set', 'sort'], read_properties: ['get', 'has'], interceptors: { - set: (notify_read_methods, value, property, ...params) => { + set: (notify_read_properties, value, property, ...params) => { if (typeof params[0] == 'string' && value.get(params[0]) === params[1]) { return false; } - notify_read_methods(['get', 'has'], params[0]); + notify_read_properties(['get', 'has'], params[0]); return true; }, - append: (notify_read_methods, value, property, ...params) => { + append: (notify_read_properties, value, property, ...params) => { if (typeof params[0] == 'string' && value.get(params[0]) === params[1]) { return false; } - notify_read_methods(['get', 'has'], params[0]); + notify_read_properties(['get', 'has'], params[0]); return true; }, - delete: (notify_read_methods, value, property, ...params) => { + delete: (notify_read_properties, value, property, ...params) => { if (typeof params[0] == 'string' && !value.has(params[0])) { return false; } - notify_read_methods(['get', 'has'], params[0]); + notify_read_properties(['get', 'has'], params[0]); return true; } } diff --git a/packages/svelte/src/reactivity/url.js b/packages/svelte/src/reactivity/url.js index 1f5e8facf39c..8a21e9b39a17 100644 --- a/packages/svelte/src/reactivity/url.js +++ b/packages/svelte/src/reactivity/url.js @@ -1,6 +1,10 @@ import { ReactiveURLSearchParams } from './url-search-params.js'; import { make_reactive } from './utils.js'; +/** + * had to create a subclass for URLWithReactiveSearchParams + * because we cannot change the internal `searchParams` reference (which links to the web api implementation) so it requires modifications + */ class URLWithReactiveSearchParams extends URL { /** * @type {InstanceType} @@ -106,72 +110,72 @@ export const ReactiveURL = make_reactive(URLWithReactiveSearchParams, { ], read_properties: ['href', 'origin', 'host', 'hash', 'pathname'], interceptors: { - protocol: (notify_read_methods, value, property, ...params) => { + protocol: (notify_read_properties, value, property, ...params) => { if ( typeof params[0] == 'string' && value.protocol.split(':')[0] === params[0].split(':')[0] ) { return false; } - notify_read_methods(['href', 'origin']); + notify_read_properties(['href', 'origin']); return true; }, - username: (notify_read_methods, value, property, ...params) => { + username: (notify_read_properties, value, property, ...params) => { if (value.protocol === params[0]) { return false; } - notify_read_methods(['href']); + notify_read_properties(['href']); return true; }, - password: (notify_read_methods, value, property, ...params) => { + password: (notify_read_properties, value, property, ...params) => { if (value.protocol === params[0]) { return false; } - notify_read_methods(['href']); + notify_read_properties(['href']); return true; }, - hostname: (notify_read_methods, value, property, ...params) => { + hostname: (notify_read_properties, value, property, ...params) => { if (value.protocol === params[0]) { return false; } - notify_read_methods(['href', 'host']); + notify_read_properties(['href', 'host']); return true; }, - port: (notify_read_methods, value, property, ...params) => { + port: (notify_read_properties, value, property, ...params) => { if (value.protocol === params[0]) { return false; } - notify_read_methods(['href', 'origin', 'host']); + notify_read_properties(['href', 'origin', 'host']); return true; }, - pathname: (notify_read_methods, value, property, ...params) => { + pathname: (notify_read_properties, value, property, ...params) => { if (value.protocol === params[0]) { return false; } - notify_read_methods(['href']); + notify_read_properties(['href']); return true; }, - hash: (notify_read_methods, value, property, ...params) => { + hash: (notify_read_properties, value, property, ...params) => { if (value.protocol === params[0]) { return false; } - notify_read_methods(['href']); + notify_read_properties(['href']); return true; }, - search: (notify_read_methods, value, property, ...params) => { + search: (notify_read_properties, value, property, ...params) => { if (value.search === params[0]) { return false; } - notify_read_methods(['href', 'hash']); + notify_read_properties(['href', 'hash']); return true; }, - href: (notify_read_methods, value, property, ...params) => { + href: (notify_read_properties, value, property, ...params) => { if (value.href === params[0]) { return false; } - notify_read_methods(['origin', 'host', 'hash', 'pathname']); + notify_read_properties(['origin', 'host', 'hash', 'pathname']); return true; } } diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index de6931deaec2..1ec187be3562 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -9,13 +9,13 @@ export const NOTIFY_WITH_ALL_PARAMS = Symbol(); * for instance calling `set.add(2)` two times should not cause reactivity the second time. * interceptor is called before the call is proxied to the actual object, so we can decide wether a change * is actually going to happen or not. - * - if a `write_property` shouldn't increment the `version` signal return false from the interceptor. note that calling `notify_read_methods` WILL increase the `version` in all cases. - * returning false is only useful if do it before calling `notify_read_methods` like an if-guard that returns false early because no change has happened. + * - if a `write_property` shouldn't increment the `version` signal return false from the interceptor. note that calling `notify_read_properties` WILL increase the `version` in all cases. + * returning false is only useful if do it before calling `notify_read_properties` like an if-guard that returns false early because no change has happened. * - DO NOT USE INTERCEPTORS FOR READ PROPERTIES * @template TEntityInstance * @template {(keyof TEntityInstance)[]} TWriteProperties * @template {(keyof TEntityInstance)[]} TReadProperties - * @typedef {Partialvoid ,value: TEntityInstance, property: TWriteProperties[number], ...params: unknown[])=>boolean>>} Interceptors + * @typedef {Partialvoid ,value: TEntityInstance, property: TWriteProperties[number], ...params: unknown[])=>boolean>>} Interceptors */ /** @@ -167,7 +167,7 @@ function create_notifiers( interceptor( (methods, ...params) => { notifiers.push(() => { - notify_read_methods( + notify_read_properties( options_with_version_flag, version_signal, read_methods_signals, @@ -232,7 +232,7 @@ function get_read_signals(version_signal, read_methods_signals, property, option * @param {TReadProperties | undefined} method_names * @param {unknown[]} params - if you want to notify for all parameters pass the `NOTIFY_WITH_ALL_PARAMS` constant, for instance some methods like `clear` should notify all `something.get(x)` methods; on these cases set this flag to true */ -function notify_read_methods( +function notify_read_properties( options, version_signal, read_methods_signals, From 0ccda208fe15ce44fb37d36b39720aeba1dbb970 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 22 May 2024 11:28:05 +0330 Subject: [PATCH 38/71] refactored date.js to use the generic make_reactive utility --- packages/svelte/src/reactivity/date.js | 125 +++++-------------------- 1 file changed, 23 insertions(+), 102 deletions(-) diff --git a/packages/svelte/src/reactivity/date.js b/packages/svelte/src/reactivity/date.js index cf43dfe4ab43..69bb3003cdd5 100644 --- a/packages/svelte/src/reactivity/date.js +++ b/packages/svelte/src/reactivity/date.js @@ -1,102 +1,23 @@ -import { source, set } from '../internal/client/reactivity/sources.js'; -import { get } from '../internal/client/runtime.js'; - -/** @type {Array} */ -const read = [ - 'getDate', - 'getDay', - 'getFullYear', - 'getHours', - 'getMilliseconds', - 'getMinutes', - 'getMonth', - 'getSeconds', - 'getTime', - 'getTimezoneOffset', - 'getUTCDate', - 'getUTCDay', - 'getUTCFullYear', - 'getUTCHours', - 'getUTCMilliseconds', - 'getUTCMinutes', - 'getUTCMonth', - 'getUTCSeconds', - // @ts-expect-error this is deprecated - 'getYear', - 'toDateString', - 'toISOString', - 'toJSON', - 'toLocaleDateString', - 'toLocaleString', - 'toLocaleTimeString', - 'toString', - 'toTimeString', - 'toUTCString' -]; - -/** @type {Array} */ -const write = [ - 'setDate', - 'setFullYear', - 'setHours', - 'setMilliseconds', - 'setMinutes', - 'setMonth', - 'setSeconds', - 'setTime', - 'setUTCDate', - 'setUTCFullYear', - 'setUTCHours', - 'setUTCMilliseconds', - 'setUTCMinutes', - 'setUTCMonth', - 'setUTCSeconds', - // @ts-expect-error this is deprecated - 'setYear' -]; - -var inited = false; - -export class ReactiveDate extends Date { - #raw_time = source(super.getTime()); - - // We init as part of the first instance so that we can treeshake this class - #init() { - if (!inited) { - inited = true; - const proto = ReactiveDate.prototype; - const date_proto = Date.prototype; - - for (const method of read) { - // @ts-ignore - proto[method] = function (...args) { - get(this.#raw_time); - // @ts-ignore - return date_proto[method].apply(this, args); - }; - } - - for (const method of write) { - // @ts-ignore - proto[method] = function (...args) { - // @ts-ignore - const v = date_proto[method].apply(this, args); - const time = date_proto.getTime.call(this); - if (time !== this.#raw_time.v) { - set(this.#raw_time, time); - } - return v; - }; - } - } - } - - /** - * @param {any[]} values - */ - constructor(...values) { - // @ts-ignore - super(...values); - this.#init(); - } -} +import { make_reactive } from './utils.js'; + +export const ReactiveDate = make_reactive(Date, { + write_properties: [ + 'setDate', + 'setFullYear', + 'setHours', + 'setMilliseconds', + 'setMinutes', + 'setMonth', + 'setSeconds', + 'setTime', + 'setUTCDate', + 'setUTCFullYear', + 'setUTCHours', + 'setUTCMilliseconds', + 'setUTCMinutes', + 'setUTCMonth', + 'setUTCSeconds', + // @ts-expect-error this is deprecated + 'setYear' + ] +}); From 989f429be21c05316f168f3315f5bfaad88981e9 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 22 May 2024 11:29:21 +0330 Subject: [PATCH 39/71] improved comments --- packages/svelte/src/reactivity/url.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/svelte/src/reactivity/url.js b/packages/svelte/src/reactivity/url.js index 8a21e9b39a17..a3e5d43570d2 100644 --- a/packages/svelte/src/reactivity/url.js +++ b/packages/svelte/src/reactivity/url.js @@ -3,7 +3,8 @@ import { make_reactive } from './utils.js'; /** * had to create a subclass for URLWithReactiveSearchParams - * because we cannot change the internal `searchParams` reference (which links to the web api implementation) so it requires modifications + * because we cannot change the internal `searchParams` reference (which links to the web api implementation) so it requires + * some custom logic */ class URLWithReactiveSearchParams extends URL { /** From 0e4239c3d1b1ad2c711b85688074670a136b3424 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 22 May 2024 11:55:58 +0330 Subject: [PATCH 40/71] added more tests for searchParams --- .../src/reactivity/url-reach-params.test.ts | 27 ---- .../src/reactivity/url-search-params.test.ts | 115 ++++++++++++++++++ 2 files changed, 115 insertions(+), 27 deletions(-) delete mode 100644 packages/svelte/src/reactivity/url-reach-params.test.ts create mode 100644 packages/svelte/src/reactivity/url-search-params.test.ts diff --git a/packages/svelte/src/reactivity/url-reach-params.test.ts b/packages/svelte/src/reactivity/url-reach-params.test.ts deleted file mode 100644 index d6937004676b..000000000000 --- a/packages/svelte/src/reactivity/url-reach-params.test.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { render_effect, effect_root } from '../internal/client/reactivity/effects.js'; -import { flushSync } from '../index-client.js'; -import { ReactiveURLSearchParams } from './url-search-params.js'; -import { assert, test } from 'vitest'; - -test('URLSearchParams', () => { - const params = new ReactiveURLSearchParams(); - const log: any = []; - - const cleanup = effect_root(() => { - render_effect(() => { - log.push(params.toString()); - }); - }); - - flushSync(() => { - params.set('a', 'b'); - }); - - flushSync(() => { - params.append('a', 'c'); - }); - - assert.deepEqual(log, ['', 'a=b', 'a=b&a=c']); - - cleanup(); -}); diff --git a/packages/svelte/src/reactivity/url-search-params.test.ts b/packages/svelte/src/reactivity/url-search-params.test.ts new file mode 100644 index 000000000000..0e60106a7b47 --- /dev/null +++ b/packages/svelte/src/reactivity/url-search-params.test.ts @@ -0,0 +1,115 @@ +import { render_effect, effect_root } from '../internal/client/reactivity/effects.js'; +import { flushSync } from '../index-client.js'; +import { ReactiveURLSearchParams } from './url-search-params.js'; +import { assert, test } from 'vitest'; + +test('URLSearchParams.set', () => { + const params = new ReactiveURLSearchParams(); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(params.toString()); + }); + }); + + flushSync(() => { + params.set('a', 'b'); + }); + + flushSync(() => { + params.set('a', 'c'); + }); + + flushSync(() => { + params.set('a', 'c'); + }); + + assert.deepEqual(log, ['', 'a=b', 'a=c']); + + cleanup(); +}); + +test('URLSearchParams.append', () => { + const params = new ReactiveURLSearchParams(); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(params.toString()); + }); + }); + + flushSync(() => { + params.append('a', 'b'); + }); + + flushSync(() => { + params.set('a', 'b'); + }); + + flushSync(() => { + params.append('a', 'c'); + }); + + assert.deepEqual(log, ['', 'a=b', 'a=b&a=c']); + + cleanup(); +}); + +test('URLSearchParams.delete', () => { + const params = new ReactiveURLSearchParams('a=b&c=d'); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(params.toString()); + }); + }); + + flushSync(() => { + params.delete('a'); + }); + + flushSync(() => { + params.delete('a'); + }); + + flushSync(() => { + params.set('a', 'b'); + }); + + assert.deepEqual(log, ['a=b&c=d', 'c=d', 'c=d&a=b']); + + cleanup(); +}); + +test('URLSearchParams.get', () => { + const params = new ReactiveURLSearchParams('a=b&c=d'); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(params.get('a')); + }); + render_effect(() => { + log.push(params.get('c')); + }); + }); + + flushSync(() => { + params.set('a', 'b'); + }); + + flushSync(() => { + params.set('a', 'new-b'); + }); + + flushSync(() => { + params.delete('a'); + }); + + assert.deepEqual(log, ['b', 'd', 'new-b', null]); + + cleanup(); +}); From 12ddf383de12e4c7c920ba8c4c88992d58111f85 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 22 May 2024 12:13:44 +0330 Subject: [PATCH 41/71] removed sending the constructor params to getters! --- packages/svelte/src/reactivity/utils.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 1ec187be3562..45b1400010c9 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -96,8 +96,7 @@ export const make_reactive = (Entity, options) => { target.#version_signal, target.#read_methods_signals, property, - options, - ...params + options ); } From 416a68e0ba2bbad742f51e66975cbe425418eca6 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 22 May 2024 12:44:11 +0330 Subject: [PATCH 42/71] made url more fine-grained --- packages/svelte/src/reactivity/url.js | 37 ++++++++++++++-------- packages/svelte/src/reactivity/url.test.ts | 4 +-- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/packages/svelte/src/reactivity/url.js b/packages/svelte/src/reactivity/url.js index a3e5d43570d2..86afe23760a3 100644 --- a/packages/svelte/src/reactivity/url.js +++ b/packages/svelte/src/reactivity/url.js @@ -77,7 +77,6 @@ class URLWithReactiveSearchParams extends URL { this.searchParams.delete(key); } }); - for (const key of super.searchParams.keys()) { // set/update keys this.searchParams.set(key, /** @type {string} */ (super.searchParams.get(key))); @@ -107,9 +106,22 @@ export const ReactiveURL = make_reactive(URLWithReactiveSearchParams, { 'pathname', 'hash', 'search', - 'href' + 'href', + 'host' + ], + read_properties: [ + 'protocol', + 'username', + 'password', + 'hostname', + 'port', + 'pathname', + 'hash', + 'search', + 'href', + 'host', + 'origin' ], - read_properties: ['href', 'origin', 'host', 'hash', 'pathname'], interceptors: { protocol: (notify_read_properties, value, property, ...params) => { if ( @@ -118,36 +130,35 @@ export const ReactiveURL = make_reactive(URLWithReactiveSearchParams, { ) { return false; } - notify_read_properties(['href', 'origin']); + notify_read_properties(['href', 'origin', 'protocol']); return true; }, username: (notify_read_properties, value, property, ...params) => { if (value.protocol === params[0]) { return false; } - notify_read_properties(['href']); + notify_read_properties(['href', 'username']); return true; }, password: (notify_read_properties, value, property, ...params) => { if (value.protocol === params[0]) { return false; } - notify_read_properties(['href']); + notify_read_properties(['href', 'password']); return true; }, hostname: (notify_read_properties, value, property, ...params) => { if (value.protocol === params[0]) { return false; } - notify_read_properties(['href', 'host']); + notify_read_properties(['href', 'host', 'hostname']); return true; }, - port: (notify_read_properties, value, property, ...params) => { if (value.protocol === params[0]) { return false; } - notify_read_properties(['href', 'origin', 'host']); + notify_read_properties(['href', 'origin', 'host', 'port']); return true; }, @@ -155,28 +166,28 @@ export const ReactiveURL = make_reactive(URLWithReactiveSearchParams, { if (value.protocol === params[0]) { return false; } - notify_read_properties(['href']); + notify_read_properties(['href', 'pathname']); return true; }, hash: (notify_read_properties, value, property, ...params) => { if (value.protocol === params[0]) { return false; } - notify_read_properties(['href']); + notify_read_properties(['href', 'hash']); return true; }, search: (notify_read_properties, value, property, ...params) => { if (value.search === params[0]) { return false; } - notify_read_properties(['href', 'hash']); + notify_read_properties(['href', 'hash', 'search']); return true; }, href: (notify_read_properties, value, property, ...params) => { if (value.href === params[0]) { return false; } - notify_read_properties(['origin', 'host', 'hash', 'pathname']); + notify_read_properties(['origin', 'host', 'hash', 'pathname', 'href']); return true; } } diff --git a/packages/svelte/src/reactivity/url.test.ts b/packages/svelte/src/reactivity/url.test.ts index 53f7796bc2ee..aa662047a521 100644 --- a/packages/svelte/src/reactivity/url.test.ts +++ b/packages/svelte/src/reactivity/url.test.ts @@ -68,10 +68,8 @@ test('url.searchParams', () => { 'q: true', 'search: ?q=kit&foo=baz&foo=qux', 'foo: baz', - 'q: true', 'search: ?q=kit', - 'foo: null', - 'q: true' + 'foo: null' ]); cleanup(); From 45025beb78c059f97e9997e7a57c07ea22ef2dbb Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 22 May 2024 13:21:01 +0330 Subject: [PATCH 43/71] fixed where updating the search wouldn't keep the new order of how they are added --- .../src/reactivity/url-search-params.js | 13 +++--- .../src/reactivity/url-search-params.test.ts | 36 +++++++++++++++ packages/svelte/src/reactivity/url.js | 44 ++++++++++++++----- packages/svelte/src/reactivity/url.test.ts | 1 - 4 files changed, 77 insertions(+), 17 deletions(-) diff --git a/packages/svelte/src/reactivity/url-search-params.js b/packages/svelte/src/reactivity/url-search-params.js index 0a55c0b249bd..3def2c7acd4b 100644 --- a/packages/svelte/src/reactivity/url-search-params.js +++ b/packages/svelte/src/reactivity/url-search-params.js @@ -2,27 +2,28 @@ import { make_reactive } from './utils.js'; export const ReactiveURLSearchParams = make_reactive(URLSearchParams, { write_properties: ['append', 'delete', 'set', 'sort'], - read_properties: ['get', 'has'], + read_properties: ['get', 'has', 'getAll'], interceptors: { set: (notify_read_properties, value, property, ...params) => { if (typeof params[0] == 'string' && value.get(params[0]) === params[1]) { return false; } - notify_read_properties(['get', 'has'], params[0]); + notify_read_properties(['get', 'has', 'getAll'], params[0]); return true; }, append: (notify_read_properties, value, property, ...params) => { - if (typeof params[0] == 'string' && value.get(params[0]) === params[1]) { - return false; + notify_read_properties(['getAll'], params[0]); + + if (typeof params[0] == 'string' && !value.has(params[0])) { + notify_read_properties(['get', 'has'], params[0]); } - notify_read_properties(['get', 'has'], params[0]); return true; }, delete: (notify_read_properties, value, property, ...params) => { if (typeof params[0] == 'string' && !value.has(params[0])) { return false; } - notify_read_properties(['get', 'has'], params[0]); + notify_read_properties(['get', 'has', 'getAll'], params[0]); return true; } } diff --git a/packages/svelte/src/reactivity/url-search-params.test.ts b/packages/svelte/src/reactivity/url-search-params.test.ts index 0e60106a7b47..d93b4b964e02 100644 --- a/packages/svelte/src/reactivity/url-search-params.test.ts +++ b/packages/svelte/src/reactivity/url-search-params.test.ts @@ -113,3 +113,39 @@ test('URLSearchParams.get', () => { cleanup(); }); + +test('URLSearchParams.getAll', () => { + const params = new ReactiveURLSearchParams('a=b&c=d'); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(params.get('a')); + }); + render_effect(() => { + log.push(params.get('c')); + }); + render_effect(() => { + log.push(params.get('q')); + }); + render_effect(() => { + log.push(params.getAll('a')); + }); + render_effect(() => { + log.push(params.getAll('q')); + }); + }); + + flushSync(() => { + // this shouldn't affect params.get(a) because it already exists + params.append('a', 'b1'); + }); + + flushSync(() => { + params.append('q', 'z'); + }); + + assert.deepEqual(log, ['b', 'd', null, ['b'], [], ['b', 'b1'], 'z', ['z']]); + + cleanup(); +}); diff --git a/packages/svelte/src/reactivity/url.js b/packages/svelte/src/reactivity/url.js index 86afe23760a3..d2b2b517f877 100644 --- a/packages/svelte/src/reactivity/url.js +++ b/packages/svelte/src/reactivity/url.js @@ -1,3 +1,4 @@ +import { untrack } from '../index-client.js'; import { ReactiveURLSearchParams } from './url-search-params.js'; import { make_reactive } from './utils.js'; @@ -71,21 +72,44 @@ class URLWithReactiveSearchParams extends URL { } if (url_updated) { - Array.from(this.searchParams.keys()).forEach((key) => { - // remove keys that don't exist anymore - if (!super.searchParams.has(key)) { - this.searchParams.delete(key); - } - }); - for (const key of super.searchParams.keys()) { - // set/update keys - this.searchParams.set(key, /** @type {string} */ (super.searchParams.get(key))); - } + this.#update_search_params_from_url(); } else if (search_params_updated) { + // updating url from params this.search = this.searchParams.toString(); } } + #update_search_params_from_url() { + /** + * keeping track of this is required because we have to keep the order in which they are updated + * @type {string[]} + */ + const keys_with_no_change = []; + + // remove keys that don't exist anymore and notify others + for (const [key, value] of Array.from(this.searchParams.entries())) { + if (!super.searchParams.has(key) || value == super.searchParams.get(key)) { + keys_with_no_change.push(key); + untrack(() => { + this.searchParams.delete(key); + }); + continue; + } + this.searchParams.delete(key); + } + + // set or update keys based on the params + for (const [key, value] of super.searchParams.entries()) { + if (keys_with_no_change.includes(key)) { + untrack(() => { + this.searchParams.set(key, value); + }); + continue; + } + this.searchParams.set(key, value); + } + } + /** * @override */ diff --git a/packages/svelte/src/reactivity/url.test.ts b/packages/svelte/src/reactivity/url.test.ts index aa662047a521..da88358f8cdf 100644 --- a/packages/svelte/src/reactivity/url.test.ts +++ b/packages/svelte/src/reactivity/url.test.ts @@ -67,7 +67,6 @@ test('url.searchParams', () => { 'foo: baz', 'q: true', 'search: ?q=kit&foo=baz&foo=qux', - 'foo: baz', 'search: ?q=kit', 'foo: null' ]); From 8ddf0618e5e338b42d8de4c19bb708446f21d357 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 22 May 2024 13:42:13 +0330 Subject: [PATCH 44/71] added more tests for url --- .../src/reactivity/url-search-params.test.ts | 4 ++ packages/svelte/src/reactivity/url.test.ts | 37 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/packages/svelte/src/reactivity/url-search-params.test.ts b/packages/svelte/src/reactivity/url-search-params.test.ts index d93b4b964e02..12ab7fbc476f 100644 --- a/packages/svelte/src/reactivity/url-search-params.test.ts +++ b/packages/svelte/src/reactivity/url-search-params.test.ts @@ -22,6 +22,7 @@ test('URLSearchParams.set', () => { }); flushSync(() => { + // nothing should happen here params.set('a', 'c'); }); @@ -45,6 +46,7 @@ test('URLSearchParams.append', () => { }); flushSync(() => { + // nothing should happen here params.set('a', 'b'); }); @@ -72,6 +74,7 @@ test('URLSearchParams.delete', () => { }); flushSync(() => { + // nothing should happen here params.delete('a'); }); @@ -123,6 +126,7 @@ test('URLSearchParams.getAll', () => { log.push(params.get('a')); }); render_effect(() => { + // this should only logged once because other changes shouldn't affect this log.push(params.get('c')); }); render_effect(() => { diff --git a/packages/svelte/src/reactivity/url.test.ts b/packages/svelte/src/reactivity/url.test.ts index da88358f8cdf..174a76778102 100644 --- a/packages/svelte/src/reactivity/url.test.ts +++ b/packages/svelte/src/reactivity/url.test.ts @@ -73,3 +73,40 @@ test('url.searchParams', () => { cleanup(); }); + +test('url.href', () => { + const url = new ReactiveURL('https://svelte.dev?foo=bar&t=123'); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(url.href); + }); + }); + + flushSync(() => { + url.search = '?q=kit&foo=baz'; + }); + + flushSync(() => { + url.searchParams.append('foo', 'qux'); + }); + + flushSync(() => { + url.searchParams.delete('foo'); + }); + + flushSync(() => { + url.searchParams.set('love', 'svelte5'); + }); + + assert.deepEqual(log, [ + 'https://svelte.dev/?foo=bar&t=123', + 'https://svelte.dev/?q=kit&foo=baz', + 'https://svelte.dev/?q=kit&foo=baz&foo=qux', + 'https://svelte.dev/?q=kit', + 'https://svelte.dev/?q=kit&love=svelte5' + ]); + + cleanup(); +}); From 7f3634e9378d4c0199ae6c0d138d53500db21774 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 22 May 2024 14:07:27 +0330 Subject: [PATCH 45/71] ran build and updated generated index.d.ts --- packages/svelte/types/index.d.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 9dbb2492e0fa..6e9ba6435a8f 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -2074,14 +2074,15 @@ declare module 'svelte/motion' { } declare module 'svelte/reactivity' { - class ReactiveDate extends Date { - - constructor(...values: any[]); - #private; - } + export const Date: DateConstructor; export const Set: SetConstructor; export const Map: MapConstructor; const ReactiveURL: typeof URLWithReactiveSearchParams; + /** + * had to create a subclass for URLWithReactiveSearchParams + * because we cannot change the internal `searchParams` reference (which links to the web api implementation) so it requires + * some custom logic + */ class URLWithReactiveSearchParams extends URL { #private; } @@ -2090,7 +2091,7 @@ declare module 'svelte/reactivity' { prototype: URLSearchParams; }; - export { ReactiveDate as Date, ReactiveURL as URL, ReactiveURLSearchParams as URLSearchParams }; + export { ReactiveURL as URL, ReactiveURLSearchParams as URLSearchParams }; } declare module 'svelte/server' { From 82f62e3fa86a4e4c9a39f281cb173c72a3c1008c Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 22 May 2024 15:16:01 +0330 Subject: [PATCH 46/71] fixed copy pasta errors --- packages/svelte/src/reactivity/url.js | 61 +++++++++++++++++---------- 1 file changed, 39 insertions(+), 22 deletions(-) diff --git a/packages/svelte/src/reactivity/url.js b/packages/svelte/src/reactivity/url.js index d2b2b517f877..2e20cee3539f 100644 --- a/packages/svelte/src/reactivity/url.js +++ b/packages/svelte/src/reactivity/url.js @@ -34,7 +34,7 @@ class URLWithReactiveSearchParams extends URL { */ get search() { this.searchParams.toString(); - this.#sync_params_with_url({ search_params_updated: true }); + this.#sync_params_with_url('search_params'); return super.search; } @@ -43,7 +43,7 @@ class URLWithReactiveSearchParams extends URL { */ set search(value) { super.search = value; - this.#sync_params_with_url({ url_updated: true }); + this.#sync_params_with_url('url'); } /** @@ -51,7 +51,7 @@ class URLWithReactiveSearchParams extends URL { */ get href() { this.searchParams.toString(); - this.#sync_params_with_url({ search_params_updated: true }); + this.#sync_params_with_url('search_params'); return super.href; } @@ -60,20 +60,20 @@ class URLWithReactiveSearchParams extends URL { */ set href(value) { super.href = value; - this.#sync_params_with_url({ url_updated: true }); + this.#sync_params_with_url('url'); } /** - * @param {{url_updated?: boolean, search_params_updated?: boolean}} param0 + * @param {"url" | "search_params"} changed_value */ - #sync_params_with_url({ url_updated = false, search_params_updated = false }) { + #sync_params_with_url(changed_value) { if (super.searchParams.toString() === this.searchParams.toString()) { return; } - if (url_updated) { + if (changed_value == 'url') { this.#update_search_params_from_url(); - } else if (search_params_updated) { + } else { // updating url from params this.search = this.searchParams.toString(); } @@ -115,11 +115,29 @@ class URLWithReactiveSearchParams extends URL { */ toString() { this.searchParams.toString(); - this.#sync_params_with_url({ search_params_updated: true }); + this.#sync_params_with_url('search_params'); return super.toString(); } } +/** + * @param {unknown} value + * @param {string} character + * @param {"append" | "prepend"} mode + * @returns {unknown} + */ +function add_character_if_not_exists(value, character, mode) { + if (!value || typeof value !== 'string') { + return value; + } + + if (mode == 'append') { + return value.endsWith(character) ? value : `${value}${character}`; + } + + return value.startsWith(character) ? value : `${character}${value}`; +} + export const ReactiveURL = make_reactive(URLWithReactiveSearchParams, { write_properties: [ 'protocol', @@ -131,7 +149,8 @@ export const ReactiveURL = make_reactive(URLWithReactiveSearchParams, { 'hash', 'search', 'href', - 'host' + 'host', + 'searchParams' ], read_properties: [ 'protocol', @@ -144,42 +163,40 @@ export const ReactiveURL = make_reactive(URLWithReactiveSearchParams, { 'search', 'href', 'host', - 'origin' + 'origin', + 'searchParams' ], interceptors: { protocol: (notify_read_properties, value, property, ...params) => { - if ( - typeof params[0] == 'string' && - value.protocol.split(':')[0] === params[0].split(':')[0] - ) { + if (value.protocol == add_character_if_not_exists(params[0], ':', 'append')) { return false; } notify_read_properties(['href', 'origin', 'protocol']); return true; }, username: (notify_read_properties, value, property, ...params) => { - if (value.protocol === params[0]) { + if (value.username === params[0]) { return false; } notify_read_properties(['href', 'username']); return true; }, password: (notify_read_properties, value, property, ...params) => { - if (value.protocol === params[0]) { + if (value.password === params[0]) { return false; } notify_read_properties(['href', 'password']); return true; }, hostname: (notify_read_properties, value, property, ...params) => { - if (value.protocol === params[0]) { + if (value.hostname === params[0]) { return false; } notify_read_properties(['href', 'host', 'hostname']); return true; }, port: (notify_read_properties, value, property, ...params) => { - if (value.protocol === params[0]) { + if (value.port === params[0]?.toString()) { return false; } notify_read_properties(['href', 'origin', 'host', 'port']); @@ -187,21 +204,21 @@ export const ReactiveURL = make_reactive(URLWithReactiveSearchParams, { }, pathname: (notify_read_properties, value, property, ...params) => { - if (value.protocol === params[0]) { + if (value.pathname === add_character_if_not_exists(params[0], '/', 'prepend')) { return false; } notify_read_properties(['href', 'pathname']); return true; }, hash: (notify_read_properties, value, property, ...params) => { - if (value.protocol === params[0]) { + if (value.hash === add_character_if_not_exists(params[0], '#', 'prepend')) { return false; } notify_read_properties(['href', 'hash']); return true; }, search: (notify_read_properties, value, property, ...params) => { - if (value.search === params[0]) { + if (value.search === add_character_if_not_exists(params[0], '?', 'prepend')) { return false; } notify_read_properties(['href', 'hash', 'search']); From f79784a374bf7efaa77edc32f5febacacd18a3ee Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 22 May 2024 15:56:28 +0330 Subject: [PATCH 47/71] added fine-grained test for url --- packages/svelte/src/reactivity/url.js | 21 +++- packages/svelte/src/reactivity/url.test.ts | 122 +++++++++++++++++++++ 2 files changed, 142 insertions(+), 1 deletion(-) diff --git a/packages/svelte/src/reactivity/url.js b/packages/svelte/src/reactivity/url.js index 2e20cee3539f..6660152f803f 100644 --- a/packages/svelte/src/reactivity/url.js +++ b/packages/svelte/src/reactivity/url.js @@ -228,7 +228,26 @@ export const ReactiveURL = make_reactive(URLWithReactiveSearchParams, { if (value.href === params[0]) { return false; } - notify_read_properties(['origin', 'host', 'hash', 'pathname', 'href']); + const new_url = new URL(value); + new_url.href = /**@type {string}*/ (params[0]); + if (new_url.origin !== value.origin) { + notify_read_properties(['origin']); + } + if (new_url.host !== value.host) { + notify_read_properties(['host']); + } + if (new_url.hash !== value.hash) { + notify_read_properties(['hash']); + } + if (new_url.pathname !== value.pathname) { + notify_read_properties(['pathname']); + } + if (new_url.protocol !== value.protocol) { + notify_read_properties(['protocol']); + } + if (new_url.href !== value.href) { + notify_read_properties(['href']); + } return true; } } diff --git a/packages/svelte/src/reactivity/url.test.ts b/packages/svelte/src/reactivity/url.test.ts index 174a76778102..8068f66b8910 100644 --- a/packages/svelte/src/reactivity/url.test.ts +++ b/packages/svelte/src/reactivity/url.test.ts @@ -110,3 +110,125 @@ test('url.href', () => { cleanup(); }); + +test('fine grained tests', () => { + const url = new ReactiveURL('https://svelte.dev/'); + + let changes: Record = { + hash: true, + host: true, + hostname: true, + href: true, + origin: true, + username: true, + password: true, + pathname: true, + port: true, + protocol: true, + search: true, + searchParams: true, + toJSON: true, + toString: true + }; + + const reset_change = () => { + for (const key of Object.keys(changes) as (keyof typeof url)[]) { + changes[key] = false; + } + }; + + const cleanup = effect_root(() => { + render_effect(() => { + url.hash; + assert.equal(changes.hash, true); + }); + + render_effect(() => { + url.host; + assert.equal(changes.host, true); + }); + + render_effect(() => { + url.hostname; + assert.equal(changes.hostname, true); + }); + + render_effect(() => { + url.href; + assert.equal(changes.href, true); + }); + + render_effect(() => { + url.origin; + assert.equal(changes.origin, true); + }); + + render_effect(() => { + url.search; + assert.equal(changes.search, true); + }); + + render_effect(() => { + url.searchParams.get('fohoov'); + assert.equal(changes.searchParams, true); + }); + + render_effect(() => { + url; + reset_change(); + }); + }); + + flushSync(() => { + changes = { ...changes, origin: true, host: true, pathname: true, href: true }; + url.href = 'https://www.google.com/test'; + }); + + flushSync(() => { + changes = { ...changes, origin: true, href: true }; + url.protocol = 'http'; + }); + + flushSync(() => { + url.protocol = 'http'; + }); + + flushSync(() => { + changes = { ...changes, hash: true, href: true }; + url.hash = 'new-hash'; + }); + + flushSync(() => { + url.hash = 'new-hash'; + }); + + flushSync(() => { + changes = { ...changes, hostname: true, href: true }; + url.hostname = 'fohoov'; + }); + + flushSync(() => { + changes = { ...changes, search: true, searchParams: true }; + url.search = 'fohoov=true'; + }); + + flushSync(() => { + url.search = 'fohoov=true'; + }); + + flushSync(() => { + changes = { ...changes, search: true, searchParams: true }; + url.search = 'fohoov=false'; + }); + + flushSync(() => { + changes = { ...changes, search: true, searchParams: true }; + url.search = ''; + }); + + flushSync(() => { + url.search = ''; + }); + + cleanup(); +}); From ab13b7ff4003cf44f8e93fac07f29e35331ce718 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 22 May 2024 16:31:37 +0330 Subject: [PATCH 48/71] removed searchParams on url from write_properties --- packages/svelte/src/reactivity/url.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/svelte/src/reactivity/url.js b/packages/svelte/src/reactivity/url.js index 6660152f803f..b878f160d2c8 100644 --- a/packages/svelte/src/reactivity/url.js +++ b/packages/svelte/src/reactivity/url.js @@ -149,8 +149,7 @@ export const ReactiveURL = make_reactive(URLWithReactiveSearchParams, { 'hash', 'search', 'href', - 'host', - 'searchParams' + 'host' ], read_properties: [ 'protocol', From c9a9881e41b25a96023a347300c6afaacbbf272f Mon Sep 17 00:00:00 2001 From: godzylinux Date: Wed, 22 May 2024 20:45:21 +0330 Subject: [PATCH 49/71] used version_signal's initial value to determine if it has already changed within a single changeset or not --- packages/svelte/src/reactivity/url.test.ts | 2 +- packages/svelte/src/reactivity/utils.js | 34 ++++++++++++---------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/packages/svelte/src/reactivity/url.test.ts b/packages/svelte/src/reactivity/url.test.ts index 8068f66b8910..ea1b1accca9a 100644 --- a/packages/svelte/src/reactivity/url.test.ts +++ b/packages/svelte/src/reactivity/url.test.ts @@ -132,7 +132,7 @@ test('fine grained tests', () => { }; const reset_change = () => { - for (const key of Object.keys(changes) as (keyof typeof url)[]) { + for (const key of Object.keys(changes) as Array) { changes[key] = false; } }; diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 45b1400010c9..371cfcda1ad8 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -24,7 +24,9 @@ export const NOTIFY_WITH_ALL_PARAMS = Symbol(); * @template {(keyof TEntityInstance)[]} TReadProperties * @typedef {object} Options * @prop {TWriteProperties} write_properties - an array of property names on `TEntityInstance`, could cause reactivity. - * @prop {TReadProperties} [read_properties] - an array of property names on `TEntityInstance` that `write_properties` affect, typically used for methods. for instance `size` doesn't need to be here because it takes no parameters and is reactive based on the `version` signal. + * @prop {TReadProperties} [read_properties] - an array of property names on `TEntityInstance` that `write_properties` affect, + * typically used for methods. note that properties that are listed here don't depend on version_signal anymore and you have to + * notify changes yourself. for instance `size` doesn't need to be here because it takes no parameters and is reactive based on the `version` signal. * @prop {Interceptors} [interceptors] - an object of interceptors for `write_properties` that can customize how/when a `read_properties` should be notified of a change. */ @@ -148,8 +150,7 @@ function create_notifiers( options, ...params ) { - // the `version_signal_incremented` flag helps to update the `version_signal` only once - const options_with_version_flag = { ...options, is_version_signal_incremented: false }; + const initial_version_signal_v = version_signal.v; /** * @type {Function[]} @@ -167,8 +168,9 @@ function create_notifiers( (methods, ...params) => { notifiers.push(() => { notify_read_properties( - options_with_version_flag, + options, version_signal, + initial_version_signal_v, read_methods_signals, methods, ...params @@ -187,7 +189,7 @@ function create_notifiers( notifiers.push(() => { if (options.write_properties.some((v) => v === property)) { - increment_signal(options_with_version_flag, version_signal); + increment_signal(initial_version_signal_v, version_signal); } }); @@ -210,7 +212,8 @@ function create_notifiers( function get_read_signals(version_signal, read_methods_signals, property, options, ...params) { if (options.read_properties?.includes(property)) { (params.length == 0 ? [null] : params).forEach((param) => { - // read like methods that are reactive conditionally should create the signal (if not already created) so they can be reactive when notified based on their param + // read like methods that are reactive conditionally should create the signal (if not already created) + // so they can be reactive when notified based on their param const sig = get_signal_for_function(read_methods_signals, property, param, true); get(sig); }); @@ -225,15 +228,17 @@ function get_read_signals(version_signal, read_methods_signals, property, option * @template {(keyof TEntityInstance)[]} TWriteProperties * @template {(keyof TEntityInstance)[]} TReadProperties * @template {InstanceType} TEntityInstance - * @param {import('#client').Source} version_signal * @param {ReadMethodsSignals} read_methods_signals - * @param {Options, TWriteProperties, TReadProperties> & {is_version_signal_incremented: boolean}} options + * @param {Options, TWriteProperties, TReadProperties>} options + * @param {import('#client').Source} version_signal + * @param {boolean} initial_version_signal_v * @param {TReadProperties | undefined} method_names - * @param {unknown[]} params - if you want to notify for all parameters pass the `NOTIFY_WITH_ALL_PARAMS` constant, for instance some methods like `clear` should notify all `something.get(x)` methods; on these cases set this flag to true + * @param {unknown[]} params - if you want to notify for all parameters pass the `NOTIFY_WITH_ALL_PARAMS` constant instead, for instance some methods like `clear` should notify all `something.get(x)` methods; */ function notify_read_properties( options, version_signal, + initial_version_signal_v, read_methods_signals, method_names, ...params @@ -246,12 +251,12 @@ function notify_read_properties( } if (params.length == 1 && params[0] == NOTIFY_WITH_ALL_PARAMS) { read_methods_signals.get(name)?.forEach((sig) => { - increment_signal(options, version_signal, sig); + increment_signal(initial_version_signal_v, version_signal, sig); }); } else { (params.length == 0 ? [null] : params).forEach((param) => { const sig = get_signal_for_function(read_methods_signals, name, param); - sig && increment_signal(options, version_signal, sig); + sig && increment_signal(initial_version_signal_v, version_signal, sig); }); } }); @@ -312,13 +317,12 @@ const get_signal_for_function = ( /** * toggles the signal value. this change notifies any reactions (not using number explicitly cause its not required, change from true to false or vice versa is enough). - * @param {{is_version_signal_incremented: boolean}} options - this prevents changing the version signal multiple times in a single changeset + * @param {boolean} initial_version_signal_v - this prevents changing the version signal multiple times in a single changeset * @param {import("#client").Source} version_signal * @param {import("#client").Source} [read_method_signal] */ -const increment_signal = (options, version_signal, read_method_signal) => { - if (!options.is_version_signal_incremented) { - options.is_version_signal_incremented = true; +const increment_signal = (initial_version_signal_v, version_signal, read_method_signal) => { + if (initial_version_signal_v === version_signal.v) { set(version_signal, !version_signal.v); } read_method_signal && set(read_method_signal, !read_method_signal.v); From 376297ed273f0c5f169bb98a28232a8a5089b808 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Thu, 23 May 2024 11:09:06 +0330 Subject: [PATCH 50/71] added more tests --- packages/svelte/src/reactivity/map.test.ts | 4 ++++ packages/svelte/src/reactivity/set.test.ts | 4 ++++ packages/svelte/src/reactivity/url-search-params.test.ts | 4 ++++ packages/svelte/src/reactivity/url.test.ts | 4 ++++ 4 files changed, 16 insertions(+) diff --git a/packages/svelte/src/reactivity/map.test.ts b/packages/svelte/src/reactivity/map.test.ts index f35b1a399a0f..abd03f89a9d2 100644 --- a/packages/svelte/src/reactivity/map.test.ts +++ b/packages/svelte/src/reactivity/map.test.ts @@ -211,3 +211,7 @@ test('not invoking reactivity when value is not in the map after changes', () => cleanup(); }); + +test('Map.instanceOf', () => { + assert.equal(new ReactiveMap() instanceof Map, true); +}); diff --git a/packages/svelte/src/reactivity/set.test.ts b/packages/svelte/src/reactivity/set.test.ts index 811a99d525c7..2598794c4663 100644 --- a/packages/svelte/src/reactivity/set.test.ts +++ b/packages/svelte/src/reactivity/set.test.ts @@ -149,3 +149,7 @@ test('not invoking reactivity when value is not in the set after changes', () => cleanup(); }); + +test('Set.instanceOf', () => { + assert.equal(new ReactiveSet() instanceof Set, true); +}); diff --git a/packages/svelte/src/reactivity/url-search-params.test.ts b/packages/svelte/src/reactivity/url-search-params.test.ts index 12ab7fbc476f..6673710fc169 100644 --- a/packages/svelte/src/reactivity/url-search-params.test.ts +++ b/packages/svelte/src/reactivity/url-search-params.test.ts @@ -153,3 +153,7 @@ test('URLSearchParams.getAll', () => { cleanup(); }); + +test('URLSearchParams.instanceOf', () => { + assert.equal(new ReactiveURLSearchParams() instanceof URLSearchParams, true); +}); diff --git a/packages/svelte/src/reactivity/url.test.ts b/packages/svelte/src/reactivity/url.test.ts index ea1b1accca9a..9b2d95957bcd 100644 --- a/packages/svelte/src/reactivity/url.test.ts +++ b/packages/svelte/src/reactivity/url.test.ts @@ -232,3 +232,7 @@ test('fine grained tests', () => { cleanup(); }); + +test('URL.instanceOf', () => { + assert.equal(new ReactiveURL('https://svelte.dev') instanceof URL, true); +}); From 968d0d6e0d0b8fbc1fa28f0d7f933ae9f0d83f02 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Thu, 23 May 2024 11:09:45 +0330 Subject: [PATCH 51/71] improved and simplified console.log/debug/etc behavior of Reactive[Something] classes --- packages/svelte/src/reactivity/utils.js | 57 ++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 6 deletions(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 371cfcda1ad8..e88783556ff7 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -3,6 +3,7 @@ import { source, set } from '../internal/client/reactivity/sources.js'; import { get } from '../internal/client/runtime.js'; export const NOTIFY_WITH_ALL_PARAMS = Symbol(); +export const INTERNAL_OBJECT = Symbol(); /** * some `write_properties` require a custom logic to notify a change for read properties. @@ -41,6 +42,8 @@ export const NOTIFY_WITH_ALL_PARAMS = Symbol(); * @returns {TEntity} */ export const make_reactive = (Entity, options) => { + override_console(); + // we return a class so that the caller can call it with new // @ts-ignore return class extends Entity { @@ -57,10 +60,11 @@ export const make_reactive = (Entity, options) => { */ #version_signal = source(false); + [INTERNAL_OBJECT] = this; + /** * @param {...unknown[]} params */ - constructor(...params) { super(...params); return new Proxy(this, { @@ -271,13 +275,13 @@ function notify_read_properties( * @param {TCreateSignalIfDoesntExist} [create_signal_if_doesnt_exist=false] * @returns {TCreateSignalIfDoesntExist extends true ? import("#client").Source : import("#client").Source | null } */ -const get_signal_for_function = ( +function get_signal_for_function( signals_map, function_name, param, // @ts-ignore: this should be supported in jsdoc based on https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html#template but isn't? create_signal_if_doesnt_exist = false -) => { +) { /** * @type {Map>} */ @@ -313,7 +317,7 @@ const get_signal_for_function = ( } // @ts-ignore return signal; -}; +} /** * toggles the signal value. this change notifies any reactions (not using number explicitly cause its not required, change from true to false or vice versa is enough). @@ -321,9 +325,50 @@ const get_signal_for_function = ( * @param {import("#client").Source} version_signal * @param {import("#client").Source} [read_method_signal] */ -const increment_signal = (initial_version_signal_v, version_signal, read_method_signal) => { +function increment_signal(initial_version_signal_v, version_signal, read_method_signal) { if (initial_version_signal_v === version_signal.v) { set(version_signal, !version_signal.v); } read_method_signal && set(read_method_signal, !read_method_signal.v); -}; +} + +let is_console_overridden = false; +function override_console() { + if (!DEV || is_console_overridden) { + return; + } + is_console_overridden = true; + + const methods = /** @type {const} */ (['log', 'error', 'warn', 'debug', 'dir', 'table']); + + /** + * @param {any} value + * @returns {any} + */ + function get_internal_obj(value) { + if (typeof value === 'object' && value !== null && INTERNAL_OBJECT in value) { + return value[INTERNAL_OBJECT]; + } + return value; + } + + /** + * @param {typeof methods[number]} method + */ + function override(method) { + // eslint-disable-next-line no-console + const original_method = console[method]; + + /** + * @type {any[]} + **/ + // eslint-disable-next-line no-console + console[method] = (...params) => { + return original_method(...params.map((value) => get_internal_obj(value))); + }; + } + + methods.forEach((method) => { + override(method); + }); +} From 353184c2ba93c3c899ab68ed91a0c7f3a5de0176 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Thu, 23 May 2024 15:17:42 +0330 Subject: [PATCH 52/71] renamed NOTIFY_WITH_ALL_PARAMS to NOTIFY_WITH_ALL_REGISTERED_PARAMS --- packages/svelte/src/reactivity/map.js | 4 ++-- packages/svelte/src/reactivity/set.js | 4 ++-- packages/svelte/src/reactivity/utils.js | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/svelte/src/reactivity/map.js b/packages/svelte/src/reactivity/map.js index b1a6dac23abc..d481728cd973 100644 --- a/packages/svelte/src/reactivity/map.js +++ b/packages/svelte/src/reactivity/map.js @@ -1,4 +1,4 @@ -import { make_reactive, NOTIFY_WITH_ALL_PARAMS } from './utils.js'; +import { make_reactive, NOTIFY_WITH_ALL_REGISTERED_PARAMS } from './utils.js'; export const ReactiveMap = make_reactive(Map, { write_properties: ['clear', 'delete', 'set'], @@ -18,7 +18,7 @@ export const ReactiveMap = make_reactive(Map, { if (value.size === 0) { return false; } - notify_read_properties(['get', 'keys', 'has'], NOTIFY_WITH_ALL_PARAMS); + notify_read_properties(['get', 'keys', 'has'], NOTIFY_WITH_ALL_REGISTERED_PARAMS); return true; }, delete: (notify_read_properties, value, property, ...params) => { diff --git a/packages/svelte/src/reactivity/set.js b/packages/svelte/src/reactivity/set.js index e7e69b4a1bbc..84c81fad4860 100644 --- a/packages/svelte/src/reactivity/set.js +++ b/packages/svelte/src/reactivity/set.js @@ -1,4 +1,4 @@ -import { make_reactive, NOTIFY_WITH_ALL_PARAMS } from './utils.js'; +import { make_reactive, NOTIFY_WITH_ALL_REGISTERED_PARAMS } from './utils.js'; export const ReactiveSet = make_reactive(Set, { write_properties: ['add', 'clear', 'delete'], @@ -15,7 +15,7 @@ export const ReactiveSet = make_reactive(Set, { if (value.size == 0) { return false; } - notify_read_properties(['has'], NOTIFY_WITH_ALL_PARAMS); + notify_read_properties(['has'], NOTIFY_WITH_ALL_REGISTERED_PARAMS); return true; }, delete: (notify_read_properties, value, property, ...params) => { diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index e88783556ff7..237695be095f 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -2,7 +2,7 @@ import { DEV } from 'esm-env'; import { source, set } from '../internal/client/reactivity/sources.js'; import { get } from '../internal/client/runtime.js'; -export const NOTIFY_WITH_ALL_PARAMS = Symbol(); +export const NOTIFY_WITH_ALL_REGISTERED_PARAMS = Symbol(); export const INTERNAL_OBJECT = Symbol(); /** @@ -253,7 +253,7 @@ function notify_read_properties( `when trying to notify reactions got a read method that wasn't defined in options: ${name.toString()}` ); } - if (params.length == 1 && params[0] == NOTIFY_WITH_ALL_PARAMS) { + if (params.length == 1 && params[0] == NOTIFY_WITH_ALL_REGISTERED_PARAMS) { read_methods_signals.get(name)?.forEach((sig) => { increment_signal(initial_version_signal_v, version_signal, sig); }); From 7f2b7b0680d4582a6d0b0203537966cbc05258a8 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Thu, 23 May 2024 15:24:35 +0330 Subject: [PATCH 53/71] used `//` comments so that internal stuff won't show in the index.d.ts --- packages/svelte/src/reactivity/url.js | 10 +++++----- packages/svelte/types/index.d.ts | 5 ----- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/svelte/src/reactivity/url.js b/packages/svelte/src/reactivity/url.js index b878f160d2c8..588822d38fd6 100644 --- a/packages/svelte/src/reactivity/url.js +++ b/packages/svelte/src/reactivity/url.js @@ -2,11 +2,11 @@ import { untrack } from '../index-client.js'; import { ReactiveURLSearchParams } from './url-search-params.js'; import { make_reactive } from './utils.js'; -/** - * had to create a subclass for URLWithReactiveSearchParams - * because we cannot change the internal `searchParams` reference (which links to the web api implementation) so it requires - * some custom logic - */ +// +// had to create a subclass for URLWithReactiveSearchParams +// because we cannot change the internal `searchParams` reference (which links to the web api implementation) so it requires +// some custom logic +// class URLWithReactiveSearchParams extends URL { /** * @type {InstanceType} diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 6e9ba6435a8f..24e36e042a8d 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -2078,11 +2078,6 @@ declare module 'svelte/reactivity' { export const Set: SetConstructor; export const Map: MapConstructor; const ReactiveURL: typeof URLWithReactiveSearchParams; - /** - * had to create a subclass for URLWithReactiveSearchParams - * because we cannot change the internal `searchParams` reference (which links to the web api implementation) so it requires - * some custom logic - */ class URLWithReactiveSearchParams extends URL { #private; } From 67347752006403520a3eec08dc5d9ba580000ffe Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 24 May 2024 13:39:19 +0330 Subject: [PATCH 54/71] simplified `get_signal_for_function` and added more tests --- packages/svelte/src/reactivity/map.test.ts | 27 ++++++++++++++++++++ packages/svelte/src/reactivity/set.test.ts | 24 ++++++++++++++++++ packages/svelte/src/reactivity/utils.js | 29 ++++++---------------- 3 files changed, 58 insertions(+), 22 deletions(-) diff --git a/packages/svelte/src/reactivity/map.test.ts b/packages/svelte/src/reactivity/map.test.ts index abd03f89a9d2..619be1fa730e 100644 --- a/packages/svelte/src/reactivity/map.test.ts +++ b/packages/svelte/src/reactivity/map.test.ts @@ -212,6 +212,33 @@ test('not invoking reactivity when value is not in the map after changes', () => cleanup(); }); +test('map.clear()', () => { + const map = new ReactiveMap([ + [1, 1], + [2, 2] + ]); + + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(map.has(1)); + }); + + render_effect(() => { + log.push(map.has(2)); + }); + }); + + flushSync(() => { + map.clear(); + }); + + assert.deepEqual(log, [true, true, false, false]); + + cleanup(); +}); + test('Map.instanceOf', () => { assert.equal(new ReactiveMap() instanceof Map, true); }); diff --git a/packages/svelte/src/reactivity/set.test.ts b/packages/svelte/src/reactivity/set.test.ts index 2598794c4663..de38cff78818 100644 --- a/packages/svelte/src/reactivity/set.test.ts +++ b/packages/svelte/src/reactivity/set.test.ts @@ -150,6 +150,30 @@ test('not invoking reactivity when value is not in the set after changes', () => cleanup(); }); +test('set.clear()', () => { + const set = new ReactiveSet([1, 2]); + + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(set.has(1)); + }); + + render_effect(() => { + log.push(set.has(2)); + }); + }); + + flushSync(() => { + set.clear(); + }); + + assert.deepEqual(log, [true, true, false, false]); + + cleanup(); +}); + test('Set.instanceOf', () => { assert.equal(new ReactiveSet() instanceof Set, true); }); diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 237695be095f..bfff41a39b9d 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -164,7 +164,7 @@ function create_notifiers( const interceptor = options.interceptors && Object.hasOwn(options.interceptors, property) && - options.interceptors?.[property]; + options.interceptors[property]; if (interceptor) { const increment_version_signal = @@ -212,7 +212,6 @@ function create_notifiers( * @param {Options, TWriteProperties, TReadProperties>} options * @param {unknown[]} params */ - function get_read_signals(version_signal, read_methods_signals, property, options, ...params) { if (options.read_properties?.includes(property)) { (params.length == 0 ? [null] : params).forEach((param) => { @@ -267,12 +266,12 @@ function notify_read_properties( } /** - * gets the signal for this function based on params. If the signal doesn't exist and `create_signal_if_doesnt_exist` is not set to true, it creates a new one and returns that + * gets the signal for this function based on params. * @template {boolean} [TCreateSignalIfDoesntExist=false] * @param {ReadMethodsSignals} signals_map * @param {string | symbol | number} function_name * @param {unknown} param - * @param {TCreateSignalIfDoesntExist} [create_signal_if_doesnt_exist=false] + * @param {TCreateSignalIfDoesntExist} [create_signal_if_doesnt_exist=false] - If set to true and the signal doesn't exist, it creates a new one on `signals_map` and returns that * @returns {TCreateSignalIfDoesntExist extends true ? import("#client").Source : import("#client").Source | null } */ function get_signal_for_function( @@ -282,38 +281,24 @@ function get_signal_for_function( // @ts-ignore: this should be supported in jsdoc based on https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html#template but isn't? create_signal_if_doesnt_exist = false ) { - /** - * @type {Map>} - */ - let params_to_signal_map; - if (!signals_map.has(function_name)) { + let params_to_signal_map = signals_map.get(function_name); + if (!params_to_signal_map) { if (!create_signal_if_doesnt_exist) { // @ts-ignore return null; } params_to_signal_map = new Map([[param, source(false)]]); signals_map.set(function_name, params_to_signal_map); - } else { - params_to_signal_map = /** - * @type {Map>} - */ (signals_map.get(function_name)); } - /** - * @type {import("#client").Source} - */ - let signal; - if (!params_to_signal_map.has(param)) { + let signal = params_to_signal_map.get(param); + if (!signal) { if (!create_signal_if_doesnt_exist) { // @ts-ignore return null; } signal = source(false); params_to_signal_map.set(param, signal); - } else { - signal = /** - * @type {import("#client").Source} - */ (params_to_signal_map.get(param)); } // @ts-ignore return signal; From 68b1423d33aa25d0531340ab6ef8fbc7c3ef502d Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 24 May 2024 14:10:19 +0330 Subject: [PATCH 55/71] renamed variables and created config so interceptors can be extended easily --- packages/svelte/src/reactivity/map.js | 24 ++-- packages/svelte/src/reactivity/set.js | 21 ++-- .../src/reactivity/url-search-params.js | 20 +-- packages/svelte/src/reactivity/url.js | 78 ++++++------ packages/svelte/src/reactivity/utils.js | 114 ++++++++++-------- 5 files changed, 140 insertions(+), 117 deletions(-) diff --git a/packages/svelte/src/reactivity/map.js b/packages/svelte/src/reactivity/map.js index d481728cd973..1442e419a4dd 100644 --- a/packages/svelte/src/reactivity/map.js +++ b/packages/svelte/src/reactivity/map.js @@ -4,29 +4,29 @@ export const ReactiveMap = make_reactive(Map, { write_properties: ['clear', 'delete', 'set'], read_properties: ['get', 'keys', 'has'], interceptors: { - set: (notify_read_properties, value, property, ...params) => { - if (value.get(params[0]) === params[1]) { + set: (options, ...params) => { + if (options.value.get(params[0]) === params[1]) { return false; } - if (!value.has(params[0])) { - notify_read_properties(['keys']); + if (!options.value.has(params[0])) { + options.notify_read_properties(['keys']); } - notify_read_properties(['get', 'has'], params[0]); + options.notify_read_properties(['get', 'has'], params[0]); return true; }, - clear: (notify_read_properties, value, property, ...params) => { - if (value.size === 0) { + clear: (options, ...params) => { + if (options.value.size === 0) { return false; } - notify_read_properties(['get', 'keys', 'has'], NOTIFY_WITH_ALL_REGISTERED_PARAMS); + options.notify_read_properties(['get', 'keys', 'has'], NOTIFY_WITH_ALL_REGISTERED_PARAMS); return true; }, - delete: (notify_read_properties, value, property, ...params) => { - if (!value.has(params[0])) { + delete: (options, ...params) => { + if (!options.value.has(params[0])) { return false; } - notify_read_properties(['get', 'has'], params[0]); - notify_read_properties(['keys']); + options.notify_read_properties(['get', 'has'], params[0]); + options.notify_read_properties(['keys']); return true; } } diff --git a/packages/svelte/src/reactivity/set.js b/packages/svelte/src/reactivity/set.js index 84c81fad4860..20a8971d3791 100644 --- a/packages/svelte/src/reactivity/set.js +++ b/packages/svelte/src/reactivity/set.js @@ -4,25 +4,28 @@ export const ReactiveSet = make_reactive(Set, { write_properties: ['add', 'clear', 'delete'], read_properties: ['has'], interceptors: { - add: (notify_read_properties, value, property, ...params) => { - if (value.has(params[0])) { + add: (options, ...params) => { + if (options.value.has(params[0])) { return false; } - notify_read_properties(['has'], params[0]); + options.notify_read_properties(['has'], params[0]); return true; }, - clear: (notify_read_properties, value, property, ...params) => { - if (value.size == 0) { + clear: (options, ...params) => { + if (options.value.size == 0) { return false; } - notify_read_properties(['has'], NOTIFY_WITH_ALL_REGISTERED_PARAMS); + // get_registered_params('has').forEach((param) => { + // notify_read_properties(['has'], param); + // }); + options.notify_read_properties(['has'], NOTIFY_WITH_ALL_REGISTERED_PARAMS); return true; }, - delete: (notify_read_properties, value, property, ...params) => { - if (!value.has(params[0])) { + delete: (options, ...params) => { + if (!options.value.has(params[0])) { return false; } - notify_read_properties(['has'], params[0]); + options.notify_read_properties(['has'], params[0]); return true; } } diff --git a/packages/svelte/src/reactivity/url-search-params.js b/packages/svelte/src/reactivity/url-search-params.js index 3def2c7acd4b..bb17192365ad 100644 --- a/packages/svelte/src/reactivity/url-search-params.js +++ b/packages/svelte/src/reactivity/url-search-params.js @@ -4,26 +4,26 @@ export const ReactiveURLSearchParams = make_reactive(URLSearchParams, { write_properties: ['append', 'delete', 'set', 'sort'], read_properties: ['get', 'has', 'getAll'], interceptors: { - set: (notify_read_properties, value, property, ...params) => { - if (typeof params[0] == 'string' && value.get(params[0]) === params[1]) { + set: (options, ...params) => { + if (typeof params[0] == 'string' && options.value.get(params[0]) === params[1]) { return false; } - notify_read_properties(['get', 'has', 'getAll'], params[0]); + options.notify_read_properties(['get', 'has', 'getAll'], params[0]); return true; }, - append: (notify_read_properties, value, property, ...params) => { - notify_read_properties(['getAll'], params[0]); + append: (options, ...params) => { + options.notify_read_properties(['getAll'], params[0]); - if (typeof params[0] == 'string' && !value.has(params[0])) { - notify_read_properties(['get', 'has'], params[0]); + if (typeof params[0] == 'string' && !options.value.has(params[0])) { + options.notify_read_properties(['get', 'has'], params[0]); } return true; }, - delete: (notify_read_properties, value, property, ...params) => { - if (typeof params[0] == 'string' && !value.has(params[0])) { + delete: (options, ...params) => { + if (typeof params[0] == 'string' && !options.value.has(params[0])) { return false; } - notify_read_properties(['get', 'has', 'getAll'], params[0]); + options.notify_read_properties(['get', 'has', 'getAll'], params[0]); return true; } } diff --git a/packages/svelte/src/reactivity/url.js b/packages/svelte/src/reactivity/url.js index 588822d38fd6..7cfdf7edab76 100644 --- a/packages/svelte/src/reactivity/url.js +++ b/packages/svelte/src/reactivity/url.js @@ -166,86 +166,86 @@ export const ReactiveURL = make_reactive(URLWithReactiveSearchParams, { 'searchParams' ], interceptors: { - protocol: (notify_read_properties, value, property, ...params) => { - if (value.protocol == add_character_if_not_exists(params[0], ':', 'append')) { + protocol: (options, ...params) => { + if (options.value.protocol == add_character_if_not_exists(params[0], ':', 'append')) { return false; } - notify_read_properties(['href', 'origin', 'protocol']); + options.notify_read_properties(['href', 'origin', 'protocol']); return true; }, - username: (notify_read_properties, value, property, ...params) => { - if (value.username === params[0]) { + username: (options, ...params) => { + if (options.value.username === params[0]) { return false; } - notify_read_properties(['href', 'username']); + options.notify_read_properties(['href', 'username']); return true; }, - password: (notify_read_properties, value, property, ...params) => { - if (value.password === params[0]) { + password: (options, ...params) => { + if (options.value.password === params[0]) { return false; } - notify_read_properties(['href', 'password']); + options.notify_read_properties(['href', 'password']); return true; }, - hostname: (notify_read_properties, value, property, ...params) => { - if (value.hostname === params[0]) { + hostname: (options, ...params) => { + if (options.value.hostname === params[0]) { return false; } - notify_read_properties(['href', 'host', 'hostname']); + options.notify_read_properties(['href', 'host', 'hostname']); return true; }, - port: (notify_read_properties, value, property, ...params) => { - if (value.port === params[0]?.toString()) { + port: (options, ...params) => { + if (options.value.port === params[0]?.toString()) { return false; } - notify_read_properties(['href', 'origin', 'host', 'port']); + options.notify_read_properties(['href', 'origin', 'host', 'port']); return true; }, - pathname: (notify_read_properties, value, property, ...params) => { - if (value.pathname === add_character_if_not_exists(params[0], '/', 'prepend')) { + pathname: (options, ...params) => { + if (options.value.pathname === add_character_if_not_exists(params[0], '/', 'prepend')) { return false; } - notify_read_properties(['href', 'pathname']); + options.notify_read_properties(['href', 'pathname']); return true; }, - hash: (notify_read_properties, value, property, ...params) => { - if (value.hash === add_character_if_not_exists(params[0], '#', 'prepend')) { + hash: (options, ...params) => { + if (options.value.hash === add_character_if_not_exists(params[0], '#', 'prepend')) { return false; } - notify_read_properties(['href', 'hash']); + options.notify_read_properties(['href', 'hash']); return true; }, - search: (notify_read_properties, value, property, ...params) => { - if (value.search === add_character_if_not_exists(params[0], '?', 'prepend')) { + search: (options, ...params) => { + if (options.value.search === add_character_if_not_exists(params[0], '?', 'prepend')) { return false; } - notify_read_properties(['href', 'hash', 'search']); + options.notify_read_properties(['href', 'hash', 'search']); return true; }, - href: (notify_read_properties, value, property, ...params) => { - if (value.href === params[0]) { + href: (options, ...params) => { + if (options.value.href === params[0]) { return false; } - const new_url = new URL(value); + const new_url = new URL(options.value); new_url.href = /**@type {string}*/ (params[0]); - if (new_url.origin !== value.origin) { - notify_read_properties(['origin']); + if (new_url.origin !== options.value.origin) { + options.notify_read_properties(['origin']); } - if (new_url.host !== value.host) { - notify_read_properties(['host']); + if (new_url.host !== options.value.host) { + options.notify_read_properties(['host']); } - if (new_url.hash !== value.hash) { - notify_read_properties(['hash']); + if (new_url.hash !== options.value.hash) { + options.notify_read_properties(['hash']); } - if (new_url.pathname !== value.pathname) { - notify_read_properties(['pathname']); + if (new_url.pathname !== options.value.pathname) { + options.notify_read_properties(['pathname']); } - if (new_url.protocol !== value.protocol) { - notify_read_properties(['protocol']); + if (new_url.protocol !== options.value.protocol) { + options.notify_read_properties(['protocol']); } - if (new_url.href !== value.href) { - notify_read_properties(['href']); + if (new_url.href !== options.value.href) { + options.notify_read_properties(['href']); } return true; } diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index bfff41a39b9d..42c1d93cfdff 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -5,30 +5,44 @@ import { get } from '../internal/client/runtime.js'; export const NOTIFY_WITH_ALL_REGISTERED_PARAMS = Symbol(); export const INTERNAL_OBJECT = Symbol(); +/** + * @template TEntityInstance + * @template {(keyof TEntityInstance)[]} TWriteProperties + * @template {(keyof TEntityInstance)[]} TReadProperties + * @typedef InterceptorOptions + * @property {(methods: TReadProperties, ...params: unknown[])=>void} notify_read_properties + * @property {TEntityInstance} value + * @property {TWriteProperties[number]} property + */ + /** * some `write_properties` require a custom logic to notify a change for read properties. * for instance calling `set.add(2)` two times should not cause reactivity the second time. * interceptor is called before the call is proxied to the actual object, so we can decide wether a change * is actually going to happen or not. - * - if a `write_property` shouldn't increment the `version` signal return false from the interceptor. note that calling `notify_read_properties` WILL increase the `version` in all cases. - * returning false is only useful if do it before calling `notify_read_properties` like an if-guard that returns false early because no change has happened. + * - if a `write_property` shouldn't increment the `version` signal return false from the interceptor. + * note that calling `notify_read_properties` WILL increase the `version` in all cases. + * returning false is only useful if do it before calling `notify_read_properties` like an if-guard + * that returns false early because no change has happened. * - DO NOT USE INTERCEPTORS FOR READ PROPERTIES * @template TEntityInstance * @template {(keyof TEntityInstance)[]} TWriteProperties * @template {(keyof TEntityInstance)[]} TReadProperties - * @typedef {Partialvoid ,value: TEntityInstance, property: TWriteProperties[number], ...params: unknown[])=>boolean>>} Interceptors + * @typedef {Partial, ...params: unknown[])=>boolean>>} Interceptors */ /** * @template TEntityInstance * @template {(keyof TEntityInstance)[]} TWriteProperties * @template {(keyof TEntityInstance)[]} TReadProperties - * @typedef {object} Options + * @typedef Config * @prop {TWriteProperties} write_properties - an array of property names on `TEntityInstance`, could cause reactivity. * @prop {TReadProperties} [read_properties] - an array of property names on `TEntityInstance` that `write_properties` affect, * typically used for methods. note that properties that are listed here don't depend on version_signal anymore and you have to * notify changes yourself. for instance `size` doesn't need to be here because it takes no parameters and is reactive based on the `version` signal. - * @prop {Interceptors} [interceptors] - an object of interceptors for `write_properties` that can customize how/when a `read_properties` should be notified of a change. + * @prop {Interceptors} [interceptors] + * an object of interceptors for `write_properties` + * that can customize how/when a `read_properties` should be notified of a change. */ /** @typedef {Map>>} ReadMethodsSignals */ @@ -38,18 +52,19 @@ export const INTERNAL_OBJECT = Symbol(); * @template {(keyof InstanceType)[]} TWriteProperties * @template {(keyof InstanceType)[]} TReadProperties * @param {TEntity} Entity - the entity we want to make reactive - * @param {Options, TWriteProperties, TReadProperties>} options - configurations for how reactivity works for this entity + * @param {Config, TWriteProperties, TReadProperties>} config - configurations for how reactivity works for this entity * @returns {TEntity} */ -export const make_reactive = (Entity, options) => { +export const make_reactive = (Entity, config) => { override_console(); // we return a class so that the caller can call it with new // @ts-ignore return class extends Entity { /** - * each read method can be tracked like has, get, has and etc. these props might depend on a parameter. they have to reactive based on the - * parameter they depend on. for instance if you have `set.has(2)` and then call `set.add(5)` the former shouldn't get notified. + * each read method can be tracked like has, get, has and etc. + * these props might depend on a parameter. they have to reactive based on the parameter they depend on. + * for instance if you have `set.has(2)` and then call `set.add(5)` the former shouldn't get notified. * based on that we need to store the function_name + parameter(s). * @type {ReadMethodsSignals} **/ @@ -80,7 +95,7 @@ export const make_reactive = (Entity, options) => { target.#read_methods_signals, property, /**@type {InstanceType}*/ (target), - options, + config, ...params ); const function_result = orig_property.bind(target)(...params); @@ -89,7 +104,7 @@ export const make_reactive = (Entity, options) => { target.#version_signal, target.#read_methods_signals, property, - options, + config, ...params ); notifiers.forEach((notifier) => notifier()); @@ -102,7 +117,7 @@ export const make_reactive = (Entity, options) => { target.#version_signal, target.#read_methods_signals, property, - options + config ); } @@ -114,7 +129,7 @@ export const make_reactive = (Entity, options) => { target.#read_methods_signals, property, /**@type {InstanceType}*/ (target), - options, + config, value ); const result = Reflect.set(target, property, value); @@ -142,7 +157,7 @@ export const make_reactive = (Entity, options) => { * @param {ReadMethodsSignals} read_methods_signals * @param {TProperty} property * @param {TEntityInstance} entity_instance - * @param {Options, TWriteProperties, TReadProperties>} options + * @param {Config} config * @param {unknown[]} params * @returns {Function[]} */ @@ -151,7 +166,7 @@ function create_notifiers( read_methods_signals, property, entity_instance, - options, + config, ...params ) { const initial_version_signal_v = version_signal.v; @@ -162,29 +177,31 @@ function create_notifiers( const notifiers = []; const interceptor = - options.interceptors && - Object.hasOwn(options.interceptors, property) && - options.interceptors[property]; + config.interceptors && + Object.hasOwn(config.interceptors, property) && + config.interceptors[property]; if (interceptor) { - const increment_version_signal = - interceptor( - (methods, ...params) => { - notifiers.push(() => { - notify_read_properties( - options, - version_signal, - initial_version_signal_v, - read_methods_signals, - methods, - ...params - ); - }); - }, - entity_instance, - property, - ...params - ) === true; + /** + * @type {InterceptorOptions} + */ + const interceptor_options = { + notify_read_properties: (methods, ...params) => { + notifiers.push(() => { + notify_read_properties( + config, + version_signal, + initial_version_signal_v, + read_methods_signals, + methods, + ...params + ); + }); + }, + value: entity_instance, + property: property + }; + const increment_version_signal = interceptor(interceptor_options, ...params) === true; if (!increment_version_signal) { return notifiers; @@ -192,7 +209,7 @@ function create_notifiers( } notifiers.push(() => { - if (options.write_properties.some((v) => v === property)) { + if (config.write_properties.some((v) => v === property)) { increment_signal(initial_version_signal_v, version_signal); } }); @@ -209,19 +226,20 @@ function create_notifiers( * @param {import('#client').Source} version_signal * @param {ReadMethodsSignals} read_methods_signals * @param {TProperty} property - * @param {Options, TWriteProperties, TReadProperties>} options + * @param {Config, TWriteProperties, TReadProperties>} config * @param {unknown[]} params */ -function get_read_signals(version_signal, read_methods_signals, property, options, ...params) { - if (options.read_properties?.includes(property)) { +function get_read_signals(version_signal, read_methods_signals, property, config, ...params) { + if (config.read_properties?.includes(property)) { (params.length == 0 ? [null] : params).forEach((param) => { // read like methods that are reactive conditionally should create the signal (if not already created) // so they can be reactive when notified based on their param const sig = get_signal_for_function(read_methods_signals, property, param, true); get(sig); }); - } else if (!options.write_properties.includes(property)) { - // other read like methods that are not reactive conditionally based their params and are just notified based on the version signal are here + } else if (!config.write_properties.includes(property)) { + // other read like methods that are not reactive conditionally based their params + // and are just notified based on the version signal are here get(version_signal); } } @@ -232,14 +250,16 @@ function get_read_signals(version_signal, read_methods_signals, property, option * @template {(keyof TEntityInstance)[]} TReadProperties * @template {InstanceType} TEntityInstance * @param {ReadMethodsSignals} read_methods_signals - * @param {Options, TWriteProperties, TReadProperties>} options + * @param {Config, TWriteProperties, TReadProperties>} config * @param {import('#client').Source} version_signal * @param {boolean} initial_version_signal_v * @param {TReadProperties | undefined} method_names - * @param {unknown[]} params - if you want to notify for all parameters pass the `NOTIFY_WITH_ALL_PARAMS` constant instead, for instance some methods like `clear` should notify all `something.get(x)` methods; + * @param {unknown[]} params + * if you want to notify for all parameters pass the `NOTIFY_WITH_ALL_PARAMS` constant instead + * for instance some methods like `clear` should notify all `something.get(x)` methods; */ function notify_read_properties( - options, + config, version_signal, initial_version_signal_v, read_methods_signals, @@ -247,7 +267,7 @@ function notify_read_properties( ...params ) { method_names?.forEach((name) => { - if (DEV && !options.read_properties?.includes(name)) { + if (DEV && !config.read_properties?.includes(name)) { throw new Error( `when trying to notify reactions got a read method that wasn't defined in options: ${name.toString()}` ); @@ -271,7 +291,7 @@ function notify_read_properties( * @param {ReadMethodsSignals} signals_map * @param {string | symbol | number} function_name * @param {unknown} param - * @param {TCreateSignalIfDoesntExist} [create_signal_if_doesnt_exist=false] - If set to true and the signal doesn't exist, it creates a new one on `signals_map` and returns that + * @param {TCreateSignalIfDoesntExist} [create_signal_if_doesnt_exist=false] - if set to true and the signal doesn't exist, it creates a new one on `signals_map` and returns that * @returns {TCreateSignalIfDoesntExist extends true ? import("#client").Source : import("#client").Source | null } */ function get_signal_for_function( From bcd2927b9dbf8ef2a584e3193b2d262e1cdf625c Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 24 May 2024 14:24:45 +0330 Subject: [PATCH 56/71] made set and map more fine-grained and improved tests --- packages/svelte/src/reactivity/map.js | 19 +++++++++++++++++-- packages/svelte/src/reactivity/map.test.ts | 8 ++++++-- packages/svelte/src/reactivity/set.js | 12 +++++++----- packages/svelte/src/reactivity/set.test.ts | 8 ++++++-- packages/svelte/src/reactivity/utils.js | 4 ++++ 5 files changed, 40 insertions(+), 11 deletions(-) diff --git a/packages/svelte/src/reactivity/map.js b/packages/svelte/src/reactivity/map.js index 1442e419a4dd..34ef741ca73f 100644 --- a/packages/svelte/src/reactivity/map.js +++ b/packages/svelte/src/reactivity/map.js @@ -1,4 +1,4 @@ -import { make_reactive, NOTIFY_WITH_ALL_REGISTERED_PARAMS } from './utils.js'; +import { make_reactive } from './utils.js'; export const ReactiveMap = make_reactive(Map, { write_properties: ['clear', 'delete', 'set'], @@ -18,7 +18,22 @@ export const ReactiveMap = make_reactive(Map, { if (options.value.size === 0) { return false; } - options.notify_read_properties(['get', 'keys', 'has'], NOTIFY_WITH_ALL_REGISTERED_PARAMS); + + options.get_registered_params('has')?.forEach((value, param) => { + if (!options.value.has(param)) { + return; + } + options.notify_read_properties(['has'], param); + }); + + options.get_registered_params('get')?.forEach((value, param) => { + if (!options.value.has(param)) { + return; + } + options.notify_read_properties(['get'], param); + }); + + options.notify_read_properties(['keys']); return true; }, delete: (options, ...params) => { diff --git a/packages/svelte/src/reactivity/map.test.ts b/packages/svelte/src/reactivity/map.test.ts index 619be1fa730e..fb7f14a77415 100644 --- a/packages/svelte/src/reactivity/map.test.ts +++ b/packages/svelte/src/reactivity/map.test.ts @@ -36,7 +36,7 @@ test('map.values()', () => { map.clear(); }); - assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, false, []]); + assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, []]); cleanup(); }); @@ -228,13 +228,17 @@ test('map.clear()', () => { render_effect(() => { log.push(map.has(2)); }); + + render_effect(() => { + log.push(map.has(3)); + }); }); flushSync(() => { map.clear(); }); - assert.deepEqual(log, [true, true, false, false]); + assert.deepEqual(log, [true, true, false, false, false]); cleanup(); }); diff --git a/packages/svelte/src/reactivity/set.js b/packages/svelte/src/reactivity/set.js index 20a8971d3791..42bc572c9d08 100644 --- a/packages/svelte/src/reactivity/set.js +++ b/packages/svelte/src/reactivity/set.js @@ -1,4 +1,4 @@ -import { make_reactive, NOTIFY_WITH_ALL_REGISTERED_PARAMS } from './utils.js'; +import { make_reactive } from './utils.js'; export const ReactiveSet = make_reactive(Set, { write_properties: ['add', 'clear', 'delete'], @@ -15,10 +15,12 @@ export const ReactiveSet = make_reactive(Set, { if (options.value.size == 0) { return false; } - // get_registered_params('has').forEach((param) => { - // notify_read_properties(['has'], param); - // }); - options.notify_read_properties(['has'], NOTIFY_WITH_ALL_REGISTERED_PARAMS); + options.get_registered_params('has')?.forEach((value, param) => { + if (!options.value.has(param)) { + return; + } + options.notify_read_properties(['has'], param); + }); return true; }, delete: (options, ...params) => { diff --git a/packages/svelte/src/reactivity/set.test.ts b/packages/svelte/src/reactivity/set.test.ts index de38cff78818..370e77cdeec8 100644 --- a/packages/svelte/src/reactivity/set.test.ts +++ b/packages/svelte/src/reactivity/set.test.ts @@ -30,7 +30,7 @@ test('set.values()', () => { set.clear(); }); - assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, false, []]); + assert.deepEqual(log, [5, true, [1, 2, 3, 4, 5], 4, false, [1, 2, 4, 5], 0, []]); cleanup(); }); @@ -163,13 +163,17 @@ test('set.clear()', () => { render_effect(() => { log.push(set.has(2)); }); + + render_effect(() => { + log.push(set.has(3)); + }); }); flushSync(() => { set.clear(); }); - assert.deepEqual(log, [true, true, false, false]); + assert.deepEqual(log, [true, true, false, false, false]); cleanup(); }); diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 42c1d93cfdff..e970367298c5 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -11,6 +11,7 @@ export const INTERNAL_OBJECT = Symbol(); * @template {(keyof TEntityInstance)[]} TReadProperties * @typedef InterceptorOptions * @property {(methods: TReadProperties, ...params: unknown[])=>void} notify_read_properties + * @property {(method: TReadProperties[number]) => Map> | undefined} get_registered_params * @property {TEntityInstance} value * @property {TWriteProperties[number]} property */ @@ -198,6 +199,9 @@ function create_notifiers( ); }); }, + get_registered_params: (method) => { + return read_methods_signals.get(method); + }, value: entity_instance, property: property }; From dc80bf392b701c3dacf81d954a9be92ddcb55e49 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 24 May 2024 16:17:22 +0330 Subject: [PATCH 57/71] more streamlined behavior: only returning true will increment the version signal now --- packages/svelte/src/reactivity/utils.js | 28 +++++++++++++++---------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index e970367298c5..6875b8d53db6 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -22,9 +22,7 @@ export const INTERNAL_OBJECT = Symbol(); * interceptor is called before the call is proxied to the actual object, so we can decide wether a change * is actually going to happen or not. * - if a `write_property` shouldn't increment the `version` signal return false from the interceptor. - * note that calling `notify_read_properties` WILL increase the `version` in all cases. - * returning false is only useful if do it before calling `notify_read_properties` like an if-guard - * that returns false early because no change has happened. + * - if a `write_property` should increment the `version` signal return true from the interceptor * - DO NOT USE INTERCEPTORS FOR READ PROPERTIES * @template TEntityInstance * @template {(keyof TEntityInstance)[]} TWriteProperties @@ -214,7 +212,7 @@ function create_notifiers( notifiers.push(() => { if (config.write_properties.some((v) => v === property)) { - increment_signal(initial_version_signal_v, version_signal); + increment_version_signal(initial_version_signal_v, version_signal); } }); @@ -278,12 +276,14 @@ function notify_read_properties( } if (params.length == 1 && params[0] == NOTIFY_WITH_ALL_REGISTERED_PARAMS) { read_methods_signals.get(name)?.forEach((sig) => { - increment_signal(initial_version_signal_v, version_signal, sig); + increment_signal(sig); + increment_version_signal(initial_version_signal_v, version_signal); }); } else { (params.length == 0 ? [null] : params).forEach((param) => { const sig = get_signal_for_function(read_methods_signals, name, param); - sig && increment_signal(initial_version_signal_v, version_signal, sig); + sig && increment_signal(sig); + increment_version_signal(initial_version_signal_v, version_signal); }); } }); @@ -330,15 +330,21 @@ function get_signal_for_function( /** * toggles the signal value. this change notifies any reactions (not using number explicitly cause its not required, change from true to false or vice versa is enough). + * @param {import("#client").Source} signal + */ +function increment_signal(signal) { + signal && set(signal, !signal.v); +} + +/** * @param {boolean} initial_version_signal_v - this prevents changing the version signal multiple times in a single changeset * @param {import("#client").Source} version_signal - * @param {import("#client").Source} [read_method_signal] */ -function increment_signal(initial_version_signal_v, version_signal, read_method_signal) { - if (initial_version_signal_v === version_signal.v) { - set(version_signal, !version_signal.v); +function increment_version_signal(initial_version_signal_v, version_signal) { + if (initial_version_signal_v !== version_signal.v) { + return; } - read_method_signal && set(read_method_signal, !read_method_signal.v); + set(version_signal, !version_signal.v); } let is_console_overridden = false; From df4ae4d7ee47ce9d4c6fbafbd4797d4028279309 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 24 May 2024 21:09:33 +0330 Subject: [PATCH 58/71] initial attempt to make date fine-grained --- packages/svelte/src/reactivity/date.js | 160 ++++++++++++++++++++++--- 1 file changed, 141 insertions(+), 19 deletions(-) diff --git a/packages/svelte/src/reactivity/date.js b/packages/svelte/src/reactivity/date.js index 69bb3003cdd5..cb4ce79b9613 100644 --- a/packages/svelte/src/reactivity/date.js +++ b/packages/svelte/src/reactivity/date.js @@ -1,23 +1,145 @@ import { make_reactive } from './utils.js'; +/** + * @type {(keyof Date)[]} + */ +const write_properties = /** @type {const} */ ([ + 'setDate', + 'setFullYear', + 'setHours', + 'setMilliseconds', + 'setMinutes', + 'setMonth', + 'setSeconds', + 'setTime', + 'setUTCDate', + 'setUTCFullYear', + 'setUTCHours', + 'setUTCMilliseconds', + 'setUTCMinutes', + 'setUTCMonth', + 'setUTCSeconds', + // @ts-expect-error this is deprecated + 'setYear' +]); + +/** + * @type {(keyof Date)[]} + */ +const read_properties = /** @type {const} */ ([ + 'getDate', + 'getDay', + 'getFullYear', + 'getHours', + 'getMilliseconds', + 'getMinutes', + 'getMonth', + 'getSeconds', + 'getTimezoneOffset', + 'getUTCDate', + 'getUTCDay', + 'getUTCFullYear', + 'getUTCHours', + 'getUTCMilliseconds', + 'getUTCMinutes', + 'getUTCMonth', + 'getUTCSeconds', + // @ts-expect-error this is deprecated + 'getYear', + 'toDateString', + 'toLocaleDateString', + 'toLocaleTimeString', + 'toTimeString' +]); + +/** + * @type {Record<(typeof write_properties)[number], (typeof read_properties)[number][]>} + */ +const affected_changes = { + setDate: ['getDate', 'getUTCDate', 'toDateString', 'toLocaleDateString'], + setFullYear: [ + 'getFullYear', + 'getUTCFullYear', + 'getDate', + 'getUTCDate', + 'toDateString', + 'toLocaleDateString' + ], + setHours: ['getHours', 'getUTCHours', 'toTimeString', 'toLocaleTimeString'], + setMilliseconds: ['getMilliseconds', 'getUTCMilliseconds', 'toTimeString', 'toLocaleTimeString'], + setMinutes: [ + 'getMinutes', + 'getUTCMinutes', + 'getHours', + 'getUTCHours', + 'toTimeString', + 'toLocaleTimeString' + ], + setMonth: [ + 'getMonth', + 'getUTCMonth', + 'getDate', + 'getUTCDate', + 'toDateString', + 'toLocaleDateString' + ], + setSeconds: ['getSeconds', 'getUTCSeconds', 'toTimeString', 'toLocaleTimeString'], + setTime: [ + 'toDateString', + 'toTimeString', + 'toLocaleDateString', + 'toLocaleTimeString', + 'getFullYear', + 'getMonth', + 'getDate', + 'getHours', + 'getMinutes', + 'getSeconds', + 'getMilliseconds', + 'getUTCFullYear', + 'getUTCMonth', + 'getUTCDate', + 'getUTCHours', + 'getUTCMinutes', + 'getUTCSeconds', + 'getUTCMilliseconds' + ], + setUTCDate: ['getUTCDate', 'getDate'], + setUTCFullYear: ['getUTCFullYear', 'getFullYear', 'getUTCDate', 'getDate'], + setUTCHours: ['getUTCHours', 'getHours'], + setUTCMilliseconds: ['getUTCMilliseconds', 'getMilliseconds'], + setUTCMinutes: ['getUTCMinutes', 'getMinutes', 'getUTCHours', 'getHours'], + setUTCMonth: ['getUTCMonth', 'getMonth', 'getUTCDate', 'getDate'], + setUTCSeconds: ['getUTCSeconds', 'getSeconds'], + // @ts-expect-error + setYear: ['getYear', 'getFullYear', 'toDateString'] +}; + +/** + * @typedef {import("./utils.js").Interceptors, typeof write_properties, typeof read_properties>} ReactiveDateInterceptor + */ + export const ReactiveDate = make_reactive(Date, { - write_properties: [ - 'setDate', - 'setFullYear', - 'setHours', - 'setMilliseconds', - 'setMinutes', - 'setMonth', - 'setSeconds', - 'setTime', - 'setUTCDate', - 'setUTCFullYear', - 'setUTCHours', - 'setUTCMilliseconds', - 'setUTCMinutes', - 'setUTCMonth', - 'setUTCSeconds', - // @ts-expect-error this is deprecated - 'setYear' - ] + write_properties: write_properties, + read_properties: read_properties, + // @ts-expect-error - because of `setYear` which deprecated all types are screwed so have to compromise + interceptors: { + ...write_properties.map((write_property) => { + return /** @type {import("./utils.js").Interceptors, typeof write_properties, typeof read_properties>} */ ({ + /** + * @param {import("./utils.js").InterceptorOptions, typeof write_properties, typeof read_properties>} options + * @param {unknown[]} params + **/ + [write_property]: (options, ...params) => { + if (options.value[write_property]() === params[0]) { + return false; + } + affected_changes[write_property].forEach((affected) => { + options.notify_read_properties([affected]); + }); + return true; + } + }); + }) + } }); From b5067affa232400ecb11d274d84b0dec99106270 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 24 May 2024 21:56:57 +0330 Subject: [PATCH 59/71] used the new utility to create a fine-grained ReactiveDate --- packages/svelte/src/reactivity/date.js | 339 ++++++++++++++++--------- 1 file changed, 214 insertions(+), 125 deletions(-) diff --git a/packages/svelte/src/reactivity/date.js b/packages/svelte/src/reactivity/date.js index cb4ce79b9613..3283ae71a843 100644 --- a/packages/svelte/src/reactivity/date.js +++ b/packages/svelte/src/reactivity/date.js @@ -1,145 +1,234 @@ import { make_reactive } from './utils.js'; /** - * @type {(keyof Date)[]} + * we have to create a new Date to compare, because setting `X` might or might not affect `Y` + * for instance calling `date.setMonth(55)` will also change the `date.getYear()` + * but calling `date.setMonth(1)` (assuming its not 12) will not increase the year. + * we could check all of these edge-cases but I think that might become complicated very soon and introduce more bugs + * also there is the possibility of these behaviors to change as well, + * so creating a new date and applying the change is a better idea I guess + * @param {Date} current_datetime + * @param {Date} new_datetime + * @param {import("./utils.js").InterceptorOptions["notify_read_properties"]} notify_read_properties + * @return {boolean} - returns true if any changes happened */ -const write_properties = /** @type {const} */ ([ - 'setDate', - 'setFullYear', - 'setHours', - 'setMilliseconds', - 'setMinutes', - 'setMonth', - 'setSeconds', - 'setTime', - 'setUTCDate', - 'setUTCFullYear', - 'setUTCHours', - 'setUTCMilliseconds', - 'setUTCMinutes', - 'setUTCMonth', - 'setUTCSeconds', - // @ts-expect-error this is deprecated - 'setYear' -]); +const notify_datetime_changes = (current_datetime, new_datetime, notify_read_properties) => { + let had_time_changes = false; + let had_date_changes = false; -/** - * @type {(keyof Date)[]} - */ -const read_properties = /** @type {const} */ ([ - 'getDate', - 'getDay', - 'getFullYear', - 'getHours', - 'getMilliseconds', - 'getMinutes', - 'getMonth', - 'getSeconds', - 'getTimezoneOffset', - 'getUTCDate', - 'getUTCDay', - 'getUTCFullYear', - 'getUTCHours', - 'getUTCMilliseconds', - 'getUTCMinutes', - 'getUTCMonth', - 'getUTCSeconds', - // @ts-expect-error this is deprecated - 'getYear', - 'toDateString', - 'toLocaleDateString', - 'toLocaleTimeString', - 'toTimeString' -]); + if (current_datetime.getFullYear() !== new_datetime.getFullYear()) { + notify_read_properties(['getFullYear', 'getUTCFullYear']); + had_date_changes = true; + } -/** - * @type {Record<(typeof write_properties)[number], (typeof read_properties)[number][]>} - */ -const affected_changes = { - setDate: ['getDate', 'getUTCDate', 'toDateString', 'toLocaleDateString'], - setFullYear: [ - 'getFullYear', - 'getUTCFullYear', - 'getDate', - 'getUTCDate', - 'toDateString', - 'toLocaleDateString' - ], - setHours: ['getHours', 'getUTCHours', 'toTimeString', 'toLocaleTimeString'], - setMilliseconds: ['getMilliseconds', 'getUTCMilliseconds', 'toTimeString', 'toLocaleTimeString'], - setMinutes: [ - 'getMinutes', - 'getUTCMinutes', - 'getHours', - 'getUTCHours', - 'toTimeString', - 'toLocaleTimeString' + // @ts-expect-error + if (current_datetime.getYear && current_datetime.getYear() !== new_datetime.getYear()) { + // @ts-expect-error + notify_read_properties(['getYear']); + had_date_changes = true; + } + + if (current_datetime.getMonth() !== new_datetime.getMonth()) { + notify_read_properties(['getMonth', 'getUTCMonth']); + had_date_changes = true; + } + + if (current_datetime.getDay() !== new_datetime.getDay()) { + notify_read_properties(['getDay', 'getUTCDay']); + had_date_changes = true; + } + + if (current_datetime.getHours() !== new_datetime.getHours()) { + notify_read_properties(['getHours', 'getUTCHours']); + had_time_changes = true; + } + + if (current_datetime.getMinutes() !== new_datetime.getMinutes()) { + notify_read_properties(['getMinutes', 'getUTCMinutes']); + had_time_changes = true; + } + + if (current_datetime.getSeconds() !== new_datetime.getSeconds()) { + notify_read_properties(['getSeconds', 'getUTCSeconds']); + had_time_changes = true; + } + + if (current_datetime.getMilliseconds() !== new_datetime.getMilliseconds()) { + notify_read_properties(['getMilliseconds', 'getUTCMilliseconds']); + had_time_changes = true; + } + + if (had_time_changes) { + notify_read_properties(['toTimeString', 'toLocaleTimeString']); + } + + if (had_date_changes) { + notify_read_properties(['toDateString', 'toLocaleDateString']); + } + + return had_date_changes || had_time_changes; +}; + +export const ReactiveDate = make_reactive(Date, { + write_properties: [ + 'setDate', + 'setFullYear', + 'setHours', + 'setMilliseconds', + 'setMinutes', + 'setMonth', + 'setSeconds', + 'setTime', + 'setUTCDate', + 'setUTCFullYear', + 'setUTCHours', + 'setUTCMilliseconds', + 'setUTCMinutes', + 'setUTCMonth', + 'setUTCSeconds', + // @ts-expect-error this is deprecated + 'setYear' ], - setMonth: [ - 'getMonth', - 'getUTCMonth', + read_properties: [ 'getDate', - 'getUTCDate', - 'toDateString', - 'toLocaleDateString' - ], - setSeconds: ['getSeconds', 'getUTCSeconds', 'toTimeString', 'toLocaleTimeString'], - setTime: [ - 'toDateString', - 'toTimeString', - 'toLocaleDateString', - 'toLocaleTimeString', + 'getDay', 'getFullYear', - 'getMonth', - 'getDate', 'getHours', + 'getMilliseconds', 'getMinutes', + 'getMonth', 'getSeconds', - 'getMilliseconds', - 'getUTCFullYear', - 'getUTCMonth', + 'getTimezoneOffset', 'getUTCDate', + 'getUTCDay', + 'getUTCFullYear', 'getUTCHours', + 'getUTCMilliseconds', 'getUTCMinutes', + 'getUTCMonth', 'getUTCSeconds', - 'getUTCMilliseconds' + // @ts-expect-error this is deprecated + 'getYear', + 'toDateString', + 'toLocaleDateString', + 'toTimeString', + 'toLocaleTimeString' ], - setUTCDate: ['getUTCDate', 'getDate'], - setUTCFullYear: ['getUTCFullYear', 'getFullYear', 'getUTCDate', 'getDate'], - setUTCHours: ['getUTCHours', 'getHours'], - setUTCMilliseconds: ['getUTCMilliseconds', 'getMilliseconds'], - setUTCMinutes: ['getUTCMinutes', 'getMinutes', 'getUTCHours', 'getHours'], - setUTCMonth: ['getUTCMonth', 'getMonth', 'getUTCDate', 'getDate'], - setUTCSeconds: ['getUTCSeconds', 'getSeconds'], - // @ts-expect-error - setYear: ['getYear', 'getFullYear', 'toDateString'] -}; - -/** - * @typedef {import("./utils.js").Interceptors, typeof write_properties, typeof read_properties>} ReactiveDateInterceptor - */ - -export const ReactiveDate = make_reactive(Date, { - write_properties: write_properties, - read_properties: read_properties, - // @ts-expect-error - because of `setYear` which deprecated all types are screwed so have to compromise interceptors: { - ...write_properties.map((write_property) => { - return /** @type {import("./utils.js").Interceptors, typeof write_properties, typeof read_properties>} */ ({ - /** - * @param {import("./utils.js").InterceptorOptions, typeof write_properties, typeof read_properties>} options - * @param {unknown[]} params - **/ - [write_property]: (options, ...params) => { - if (options.value[write_property]() === params[0]) { - return false; - } - affected_changes[write_property].forEach((affected) => { - options.notify_read_properties([affected]); - }); - return true; - } - }); - }) + setDate: (options, ...params) => { + const tmp = new Date(options.value); + tmp.setDate(/**@type {number}*/ (params[0])); + return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + }, + setFullYear: (options, ...params) => { + const tmp = new Date(options.value); + tmp.setFullYear( + /**@type {number}*/ (params[0]), + /**@type {number | undefined}*/ (params[1]), + /**@type {number | undefined}*/ (params[2]) + ); + return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + }, + setHours: (options, ...params) => { + const tmp = new Date(options.value); + tmp.setHours( + /**@type {number}*/ (params[0]), + /**@type {number | undefined}*/ (params[1]), + /**@type {number | undefined}*/ (params[2]), + /**@type {number | undefined}*/ (params[3]) + ); + return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + }, + setMilliseconds: (options, ...params) => { + const tmp = new Date(options.value); + tmp.setMilliseconds(/**@type {number}*/ (params[0])); + return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + }, + setMinutes: (options, ...params) => { + const tmp = new Date(options.value); + tmp.setMinutes( + /**@type {number}*/ (params[0]), + /**@type {number | undefined}*/ (params[1]), + /**@type {number | undefined}*/ (params[2]) + ); + return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + }, + setMonth: (options, ...params) => { + const tmp = new Date(options.value); + tmp.setMonth(/**@type {number}*/ (params[0]), /**@type {number | undefined}*/ (params[1])); + return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + }, + setSeconds: (options, ...params) => { + const tmp = new Date(options.value); + tmp.setSeconds(/**@type {number}*/ (params[0]), /**@type {number | undefined}*/ (params[1])); + return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + }, + setTime: (options, ...params) => { + const tmp = new Date(options.value); + tmp.setTime(/**@type {number}*/ (params[0])); + return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + }, + setUTCDate: (options, ...params) => { + const tmp = new Date(options.value); + tmp.setUTCDate(/**@type {number}*/ (params[0])); + return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + }, + setUTCFullYear: (options, ...params) => { + const tmp = new Date(options.value); + tmp.setUTCFullYear( + /**@type {number}*/ (params[0]), + /**@type {number | undefined}*/ (params[1]), + /**@type {number | undefined}*/ (params[2]) + ); + return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + }, + setUTCHours: (options, ...params) => { + const tmp = new Date(options.value); + tmp.setUTCHours( + /**@type {number}*/ (params[0]), + /**@type {number | undefined}*/ (params[1]), + /**@type {number | undefined}*/ (params[2]), + /**@type {number | undefined}*/ (params[3]) + ); + return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + }, + setUTCMilliseconds: (options, ...params) => { + const tmp = new Date(options.value); + tmp.setUTCMilliseconds(/**@type {number}*/ (params[0])); + return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + }, + setUTCMinutes: (options, ...params) => { + const tmp = new Date(options.value); + tmp.setUTCMinutes( + /**@type {number}*/ (params[0]), + /**@type {number | undefined}*/ (params[1]), + /**@type {number | undefined}*/ (params[2]) + ); + return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + }, + setUTCMonth: (options, ...params) => { + const tmp = new Date(options.value); + tmp.setUTCMonth(/**@type {number}*/ (params[0]), /**@type {number | undefined}*/ (params[1])); + return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + }, + setUTCSeconds: (options, ...params) => { + const tmp = new Date(options.value); + tmp.setUTCSeconds( + /**@type {number}*/ (params[0]), + /**@type {number | undefined}*/ (params[1]) + ); + return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + }, + // @ts-expect-error - deprecated method + setYear: (options, ...params) => { + // it might be removed from browsers + if (!options.value.getYear) { + return false; + } + const tmp = new Date(options.value); + // @ts-expect-error + tmp.setYear(/**@type {number}*/ (params[0])); + return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + } } }); From 4885739dfe0233c1c479ddafb8d51c3fd0b00936 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 24 May 2024 22:49:19 +0330 Subject: [PATCH 60/71] simplified tmp date creation for notify_datetime_changes --- packages/svelte/src/reactivity/date.js | 168 ++++++++----------------- 1 file changed, 55 insertions(+), 113 deletions(-) diff --git a/packages/svelte/src/reactivity/date.js b/packages/svelte/src/reactivity/date.js index 3283ae71a843..ecb599dca4da 100644 --- a/packages/svelte/src/reactivity/date.js +++ b/packages/svelte/src/reactivity/date.js @@ -7,66 +7,70 @@ import { make_reactive } from './utils.js'; * we could check all of these edge-cases but I think that might become complicated very soon and introduce more bugs * also there is the possibility of these behaviors to change as well, * so creating a new date and applying the change is a better idea I guess - * @param {Date} current_datetime - * @param {Date} new_datetime - * @param {import("./utils.js").InterceptorOptions["notify_read_properties"]} notify_read_properties + * @param {import("./utils.js").InterceptorOptions} options + * @param {unknown[]} params * @return {boolean} - returns true if any changes happened */ -const notify_datetime_changes = (current_datetime, new_datetime, notify_read_properties) => { - let had_time_changes = false; - let had_date_changes = false; +const notify_datetime_changes = (options, ...params) => { + let is_time_changed = false; + let is_date_changed = false; - if (current_datetime.getFullYear() !== new_datetime.getFullYear()) { - notify_read_properties(['getFullYear', 'getUTCFullYear']); - had_date_changes = true; + const new_datetime = new Date(options.value); + + // @ts-ignore + new_datetime[options.property](...params); + + if (options.value.getFullYear() !== new_datetime.getFullYear()) { + options.notify_read_properties(['getFullYear', 'getUTCFullYear']); + is_date_changed = true; } // @ts-expect-error - if (current_datetime.getYear && current_datetime.getYear() !== new_datetime.getYear()) { + if (options.value.getYear && options.value.getYear() !== new_datetime.getYear()) { // @ts-expect-error - notify_read_properties(['getYear']); - had_date_changes = true; + options.notify_read_properties(['getYear']); + is_date_changed = true; } - if (current_datetime.getMonth() !== new_datetime.getMonth()) { - notify_read_properties(['getMonth', 'getUTCMonth']); - had_date_changes = true; + if (options.value.getMonth() !== new_datetime.getMonth()) { + options.notify_read_properties(['getMonth', 'getUTCMonth']); + is_date_changed = true; } - if (current_datetime.getDay() !== new_datetime.getDay()) { - notify_read_properties(['getDay', 'getUTCDay']); - had_date_changes = true; + if (options.value.getDay() !== new_datetime.getDay()) { + options.notify_read_properties(['getDay', 'getUTCDay']); + is_date_changed = true; } - if (current_datetime.getHours() !== new_datetime.getHours()) { - notify_read_properties(['getHours', 'getUTCHours']); - had_time_changes = true; + if (options.value.getHours() !== new_datetime.getHours()) { + options.notify_read_properties(['getHours', 'getUTCHours']); + is_time_changed = true; } - if (current_datetime.getMinutes() !== new_datetime.getMinutes()) { - notify_read_properties(['getMinutes', 'getUTCMinutes']); - had_time_changes = true; + if (options.value.getMinutes() !== new_datetime.getMinutes()) { + options.notify_read_properties(['getMinutes', 'getUTCMinutes']); + is_time_changed = true; } - if (current_datetime.getSeconds() !== new_datetime.getSeconds()) { - notify_read_properties(['getSeconds', 'getUTCSeconds']); - had_time_changes = true; + if (options.value.getSeconds() !== new_datetime.getSeconds()) { + options.notify_read_properties(['getSeconds', 'getUTCSeconds']); + is_time_changed = true; } - if (current_datetime.getMilliseconds() !== new_datetime.getMilliseconds()) { - notify_read_properties(['getMilliseconds', 'getUTCMilliseconds']); - had_time_changes = true; + if (options.value.getMilliseconds() !== new_datetime.getMilliseconds()) { + options.notify_read_properties(['getMilliseconds', 'getUTCMilliseconds']); + is_time_changed = true; } - if (had_time_changes) { - notify_read_properties(['toTimeString', 'toLocaleTimeString']); + if (is_time_changed) { + options.notify_read_properties(['toTimeString', 'toLocaleTimeString']); } - if (had_date_changes) { - notify_read_properties(['toDateString', 'toLocaleDateString']); + if (is_date_changed) { + options.notify_read_properties(['toDateString', 'toLocaleDateString']); } - return had_date_changes || had_time_changes; + return is_date_changed || is_time_changed; }; export const ReactiveDate = make_reactive(Date, { @@ -116,108 +120,49 @@ export const ReactiveDate = make_reactive(Date, { ], interceptors: { setDate: (options, ...params) => { - const tmp = new Date(options.value); - tmp.setDate(/**@type {number}*/ (params[0])); - return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + return notify_datetime_changes(options, ...params); }, setFullYear: (options, ...params) => { - const tmp = new Date(options.value); - tmp.setFullYear( - /**@type {number}*/ (params[0]), - /**@type {number | undefined}*/ (params[1]), - /**@type {number | undefined}*/ (params[2]) - ); - return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + return notify_datetime_changes(options, ...params); }, setHours: (options, ...params) => { - const tmp = new Date(options.value); - tmp.setHours( - /**@type {number}*/ (params[0]), - /**@type {number | undefined}*/ (params[1]), - /**@type {number | undefined}*/ (params[2]), - /**@type {number | undefined}*/ (params[3]) - ); - return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + return notify_datetime_changes(options, ...params); }, setMilliseconds: (options, ...params) => { - const tmp = new Date(options.value); - tmp.setMilliseconds(/**@type {number}*/ (params[0])); - return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + return notify_datetime_changes(options, ...params); }, setMinutes: (options, ...params) => { - const tmp = new Date(options.value); - tmp.setMinutes( - /**@type {number}*/ (params[0]), - /**@type {number | undefined}*/ (params[1]), - /**@type {number | undefined}*/ (params[2]) - ); - return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + return notify_datetime_changes(options, ...params); }, setMonth: (options, ...params) => { - const tmp = new Date(options.value); - tmp.setMonth(/**@type {number}*/ (params[0]), /**@type {number | undefined}*/ (params[1])); - return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + return notify_datetime_changes(options, ...params); }, setSeconds: (options, ...params) => { - const tmp = new Date(options.value); - tmp.setSeconds(/**@type {number}*/ (params[0]), /**@type {number | undefined}*/ (params[1])); - return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + return notify_datetime_changes(options, ...params); }, setTime: (options, ...params) => { - const tmp = new Date(options.value); - tmp.setTime(/**@type {number}*/ (params[0])); - return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + return notify_datetime_changes(options, ...params); }, setUTCDate: (options, ...params) => { - const tmp = new Date(options.value); - tmp.setUTCDate(/**@type {number}*/ (params[0])); - return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + return notify_datetime_changes(options, ...params); }, setUTCFullYear: (options, ...params) => { - const tmp = new Date(options.value); - tmp.setUTCFullYear( - /**@type {number}*/ (params[0]), - /**@type {number | undefined}*/ (params[1]), - /**@type {number | undefined}*/ (params[2]) - ); - return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + return notify_datetime_changes(options, ...params); }, setUTCHours: (options, ...params) => { - const tmp = new Date(options.value); - tmp.setUTCHours( - /**@type {number}*/ (params[0]), - /**@type {number | undefined}*/ (params[1]), - /**@type {number | undefined}*/ (params[2]), - /**@type {number | undefined}*/ (params[3]) - ); - return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + return notify_datetime_changes(options, ...params); }, setUTCMilliseconds: (options, ...params) => { - const tmp = new Date(options.value); - tmp.setUTCMilliseconds(/**@type {number}*/ (params[0])); - return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + return notify_datetime_changes(options, ...params); }, setUTCMinutes: (options, ...params) => { - const tmp = new Date(options.value); - tmp.setUTCMinutes( - /**@type {number}*/ (params[0]), - /**@type {number | undefined}*/ (params[1]), - /**@type {number | undefined}*/ (params[2]) - ); - return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + return notify_datetime_changes(options, ...params); }, setUTCMonth: (options, ...params) => { - const tmp = new Date(options.value); - tmp.setUTCMonth(/**@type {number}*/ (params[0]), /**@type {number | undefined}*/ (params[1])); - return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + return notify_datetime_changes(options, ...params); }, setUTCSeconds: (options, ...params) => { - const tmp = new Date(options.value); - tmp.setUTCSeconds( - /**@type {number}*/ (params[0]), - /**@type {number | undefined}*/ (params[1]) - ); - return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + return notify_datetime_changes(options, ...params); }, // @ts-expect-error - deprecated method setYear: (options, ...params) => { @@ -225,10 +170,7 @@ export const ReactiveDate = make_reactive(Date, { if (!options.value.getYear) { return false; } - const tmp = new Date(options.value); - // @ts-expect-error - tmp.setYear(/**@type {number}*/ (params[0])); - return notify_datetime_changes(options.value, tmp, options.notify_read_properties); + return notify_datetime_changes(options, ...params); } } }); From 045f8a20457cafda9ddcdfa186c4f342bfdf7107 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 24 May 2024 23:10:48 +0330 Subject: [PATCH 61/71] added tests for date --- packages/svelte/src/reactivity/date.js | 5 + packages/svelte/src/reactivity/date.test.ts | 587 ++++++++++++++++++++ 2 files changed, 592 insertions(+) create mode 100644 packages/svelte/src/reactivity/date.test.ts diff --git a/packages/svelte/src/reactivity/date.js b/packages/svelte/src/reactivity/date.js index ecb599dca4da..f9305157af66 100644 --- a/packages/svelte/src/reactivity/date.js +++ b/packages/svelte/src/reactivity/date.js @@ -37,6 +37,11 @@ const notify_datetime_changes = (options, ...params) => { is_date_changed = true; } + if (options.value.getDate() !== new_datetime.getDate()) { + options.notify_read_properties(['getDate', 'getUTCDate']); + is_date_changed = true; + } + if (options.value.getDay() !== new_datetime.getDay()) { options.notify_read_properties(['getDay', 'getUTCDay']); is_date_changed = true; diff --git a/packages/svelte/src/reactivity/date.test.ts b/packages/svelte/src/reactivity/date.test.ts new file mode 100644 index 000000000000..30905f542dc5 --- /dev/null +++ b/packages/svelte/src/reactivity/date.test.ts @@ -0,0 +1,587 @@ +import { render_effect, effect_root } from '../internal/client/reactivity/effects.js'; +import { flushSync } from '../index-client.js'; +import { ReactiveDate } from './date.js'; +import { assert, test } from 'vitest'; + +const initial_date = new ReactiveDate('2023-01-01T00:00:00.000Z'); +const new_dates = [ + new Date('2024-02-02T01:01:01.001Z'), + new Date('2025-03-03T02:02:02.002Z'), + new Date('2026-04-04T03:03:03.003Z') +]; + +test('date.setDate', () => { + const date = new ReactiveDate(initial_date); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(date.getDate()); + }); + }); + + flushSync(() => { + date.setDate(new_dates[0].getDate()); + }); + + flushSync(() => { + date.setDate(new_dates[1].getDate()); + }); + + flushSync(() => { + // nothing should happen here + date.setDate(new_dates[1].getDate()); + }); + + assert.deepEqual(log, [initial_date.getDate(), new_dates[0].getDate(), new_dates[1].getDate()]); + + cleanup(); +}); + +test('date.setFullYear', () => { + const date = new ReactiveDate(initial_date); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(date.getFullYear()); + }); + }); + + flushSync(() => { + date.setFullYear(new_dates[0].getFullYear()); + }); + + flushSync(() => { + date.setFullYear(new_dates[1].getFullYear()); + }); + + flushSync(() => { + // nothing should happen here + date.setFullYear(new_dates[1].getFullYear()); + }); + + assert.deepEqual(log, [ + initial_date.getFullYear(), + new_dates[0].getFullYear(), + new_dates[1].getFullYear() + ]); + + cleanup(); +}); + +test('date.setHours', () => { + const date = new ReactiveDate(initial_date); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(date.getHours()); + }); + }); + + flushSync(() => { + date.setHours(new_dates[0].getHours()); + }); + + flushSync(() => { + date.setHours(new_dates[1].getHours()); + }); + + flushSync(() => { + // nothing should happen here + date.setHours(new_dates[1].getHours()); + }); + + assert.deepEqual(log, [ + initial_date.getHours(), + new_dates[0].getHours(), + new_dates[1].getHours() + ]); + + cleanup(); +}); + +test('date.setMilliseconds', () => { + const date = new ReactiveDate(initial_date); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(date.getMilliseconds()); + }); + }); + + flushSync(() => { + date.setMilliseconds(new_dates[0].getMilliseconds()); + }); + + flushSync(() => { + date.setMilliseconds(new_dates[1].getMilliseconds()); + }); + + flushSync(() => { + // nothing should happen here + date.setMilliseconds(new_dates[1].getMilliseconds()); + }); + + assert.deepEqual(log, [ + initial_date.getMilliseconds(), + new_dates[0].getMilliseconds(), + new_dates[1].getMilliseconds() + ]); + + cleanup(); +}); + +test('date.setMinutes', () => { + const date = new ReactiveDate(initial_date); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(date.getMinutes()); + }); + }); + + flushSync(() => { + date.setMinutes(new_dates[0].getMinutes()); + }); + + flushSync(() => { + date.setMinutes(new_dates[1].getMinutes()); + }); + + flushSync(() => { + // nothing should happen here + date.setMinutes(new_dates[1].getMinutes()); + }); + + assert.deepEqual(log, [ + initial_date.getMinutes(), + new_dates[0].getMinutes(), + new_dates[1].getMinutes() + ]); + + cleanup(); +}); + +test('date.setMonth', () => { + const date = new ReactiveDate(initial_date); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(date.getMonth()); + }); + }); + + flushSync(() => { + date.setMonth(new_dates[0].getMonth()); + }); + + flushSync(() => { + date.setMonth(new_dates[1].getMonth()); + }); + + flushSync(() => { + // nothing should happen here + date.setMonth(new_dates[1].getMonth()); + }); + + assert.deepEqual(log, [ + initial_date.getMonth(), + new_dates[0].getMonth(), + new_dates[1].getMonth() + ]); + + cleanup(); +}); + +test('date.setSeconds', () => { + const date = new ReactiveDate(initial_date); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(date.getSeconds()); + }); + }); + + flushSync(() => { + date.setSeconds(new_dates[0].getSeconds()); + }); + + flushSync(() => { + date.setSeconds(new_dates[1].getSeconds()); + }); + + flushSync(() => { + // nothing should happen here + date.setSeconds(new_dates[1].getSeconds()); + }); + + assert.deepEqual(log, [ + initial_date.getSeconds(), + new_dates[0].getSeconds(), + new_dates[1].getSeconds() + ]); + + cleanup(); +}); + +test('date.setTime', () => { + const date = new ReactiveDate(initial_date); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(date.getTime()); + }); + }); + + flushSync(() => { + date.setTime(new_dates[0].getTime()); + }); + + flushSync(() => { + date.setTime(new_dates[1].getTime()); + }); + + flushSync(() => { + // nothing should happen here + date.setTime(new_dates[1].getTime()); + }); + + assert.deepEqual(log, [initial_date.getTime(), new_dates[0].getTime(), new_dates[1].getTime()]); + + cleanup(); +}); + +test('date.setUTCDate', () => { + const date = new ReactiveDate(initial_date); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(date.getUTCDate()); + }); + }); + + flushSync(() => { + date.setUTCDate(new_dates[0].getUTCDate()); + }); + + flushSync(() => { + date.setUTCDate(new_dates[1].getUTCDate()); + }); + + flushSync(() => { + // nothing should happen here + date.setUTCDate(new_dates[1].getUTCDate()); + }); + + assert.deepEqual(log, [ + initial_date.getUTCDate(), + new_dates[0].getUTCDate(), + new_dates[1].getUTCDate() + ]); + + cleanup(); +}); + +test('date.setUTCFullYear', () => { + const date = new ReactiveDate(initial_date); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(date.getUTCFullYear()); + }); + }); + + flushSync(() => { + date.setUTCFullYear(new_dates[0].getUTCFullYear()); + }); + + flushSync(() => { + date.setUTCFullYear(new_dates[1].getUTCFullYear()); + }); + + flushSync(() => { + // nothing should happen here + date.setUTCFullYear(new_dates[1].getUTCFullYear()); + }); + + assert.deepEqual(log, [ + initial_date.getUTCFullYear(), + new_dates[0].getUTCFullYear(), + new_dates[1].getUTCFullYear() + ]); + + cleanup(); +}); + +test('date.setUTCHours', () => { + const date = new ReactiveDate(initial_date); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(date.getUTCHours()); + }); + }); + + flushSync(() => { + date.setUTCHours(new_dates[0].getUTCHours()); + }); + + flushSync(() => { + date.setUTCHours(new_dates[1].getUTCHours()); + }); + + flushSync(() => { + // nothing should happen here + date.setUTCHours(new_dates[1].getUTCHours()); + }); + + assert.deepEqual(log, [ + initial_date.getUTCHours(), + new_dates[0].getUTCHours(), + new_dates[1].getUTCHours() + ]); + + cleanup(); +}); + +test('date.setUTCMilliseconds', () => { + const date = new ReactiveDate(initial_date); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(date.getUTCMilliseconds()); + }); + }); + + flushSync(() => { + date.setUTCMilliseconds(new_dates[0].getUTCMilliseconds()); + }); + + flushSync(() => { + date.setUTCMilliseconds(new_dates[1].getUTCMilliseconds()); + }); + + flushSync(() => { + // nothing should happen here + date.setUTCMilliseconds(new_dates[1].getUTCMilliseconds()); + }); + + assert.deepEqual(log, [ + initial_date.getUTCMilliseconds(), + new_dates[0].getUTCMilliseconds(), + new_dates[1].getUTCMilliseconds() + ]); + + cleanup(); +}); + +test('date.setUTCMinutes', () => { + const date = new ReactiveDate(initial_date); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(date.getUTCMinutes()); + }); + }); + + flushSync(() => { + date.setUTCMinutes(new_dates[0].getUTCMinutes()); + }); + + flushSync(() => { + date.setUTCMinutes(new_dates[1].getUTCMinutes()); + }); + + flushSync(() => { + // nothing should happen here + date.setUTCMinutes(new_dates[1].getUTCMinutes()); + }); + + assert.deepEqual(log, [ + initial_date.getUTCMinutes(), + new_dates[0].getUTCMinutes(), + new_dates[1].getUTCMinutes() + ]); + + cleanup(); +}); + +test('date.setUTCMonth', () => { + const date = new ReactiveDate(initial_date); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(date.getUTCMonth()); + }); + }); + + flushSync(() => { + date.setUTCMonth(new_dates[0].getUTCMonth()); + }); + + flushSync(() => { + date.setUTCMonth(new_dates[1].getUTCMonth()); + }); + + flushSync(() => { + // nothing should happen here + date.setUTCMonth(new_dates[1].getUTCMonth()); + }); + + assert.deepEqual(log, [ + initial_date.getUTCMonth(), + new_dates[0].getUTCMonth(), + new_dates[1].getUTCMonth() + ]); + + cleanup(); +}); + +test('date.setUTCSeconds', () => { + const date = new ReactiveDate(initial_date); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(date.getUTCSeconds()); + }); + }); + + flushSync(() => { + date.setUTCSeconds(new_dates[0].getUTCSeconds()); + }); + + flushSync(() => { + date.setUTCSeconds(new_dates[1].getUTCSeconds()); + }); + + flushSync(() => { + // nothing should happen here + date.setUTCSeconds(new_dates[1].getUTCSeconds()); + }); + + assert.deepEqual(log, [ + initial_date.getUTCSeconds(), + new_dates[0].getUTCSeconds(), + new_dates[1].getUTCSeconds() + ]); + + cleanup(); +}); + +test('date.setYear', () => { + const date = new ReactiveDate(initial_date); + const log: any = []; + + // @ts-expect-error + if (!date.setYear) { + return; + } + const cleanup = effect_root(() => { + render_effect(() => { + // @ts-expect-error + log.push(date.getYear()); + }); + }); + + flushSync(() => { + // @ts-expect-error + date.setYear(22); + }); + + flushSync(() => { + // @ts-expect-error + date.setYear(23); + }); + + flushSync(() => { + // nothing should happen here + // @ts-expect-error + date.setYear(23); + }); + + // @ts-expect-error + assert.deepEqual(log, [initial_date.getYear(), 22, 23]); + + cleanup(); +}); + +test('date.setSeconds - edge cases', () => { + const date = new ReactiveDate(initial_date); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(date.getSeconds()); + }); + render_effect(() => { + log.push(date.getMinutes()); + }); + }); + + flushSync(() => { + date.setSeconds(60); + }); + + flushSync(() => { + date.setSeconds(61); + }); + + assert.deepEqual(log, [ + initial_date.getSeconds(), + initial_date.getMinutes(), + initial_date.getMinutes() + 1, + initial_date.getSeconds() + 1, + initial_date.getMinutes() + 2 + ]); + + cleanup(); +}); + +test('ReactiveDate propagated changes', () => { + const date = new ReactiveDate(initial_date); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(date.getSeconds()); + }); + render_effect(() => { + log.push(date.getMonth()); + }); + render_effect(() => { + log.push(date.getFullYear()); + }); + }); + + flushSync(() => { + date.setMonth(13); + }); + + assert.deepEqual(log, [ + initial_date.getSeconds(), + initial_date.getMonth(), + initial_date.getFullYear(), + 1, + 2024 + ]); + + cleanup(); +}); + +test('Date.instanceOf', () => { + assert.equal(new ReactiveDate() instanceof Date, true); +}); From 37fafabfddea6237f1b05b5ac6b2df18fd3cd112 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 24 May 2024 23:12:07 +0330 Subject: [PATCH 62/71] renamed test name --- packages/svelte/src/reactivity/url.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/reactivity/url.test.ts b/packages/svelte/src/reactivity/url.test.ts index 9b2d95957bcd..d975ea872d1b 100644 --- a/packages/svelte/src/reactivity/url.test.ts +++ b/packages/svelte/src/reactivity/url.test.ts @@ -111,7 +111,7 @@ test('url.href', () => { cleanup(); }); -test('fine grained tests', () => { +test('url fine grained tests', () => { const url = new ReactiveURL('https://svelte.dev/'); let changes: Record = { From 2e8225a85e6a44a266e1c701247265f9e76c8117 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 24 May 2024 23:44:57 +0330 Subject: [PATCH 63/71] fixed url tests --- packages/svelte/src/reactivity/url.js | 2 +- packages/svelte/src/reactivity/url.test.ts | 50 +++++++++++++++------- 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/packages/svelte/src/reactivity/url.js b/packages/svelte/src/reactivity/url.js index 7cfdf7edab76..bcff6e24476b 100644 --- a/packages/svelte/src/reactivity/url.js +++ b/packages/svelte/src/reactivity/url.js @@ -220,7 +220,7 @@ export const ReactiveURL = make_reactive(URLWithReactiveSearchParams, { if (options.value.search === add_character_if_not_exists(params[0], '?', 'prepend')) { return false; } - options.notify_read_properties(['href', 'hash', 'search']); + options.notify_read_properties(['href', 'search']); return true; }, href: (options, ...params) => { diff --git a/packages/svelte/src/reactivity/url.test.ts b/packages/svelte/src/reactivity/url.test.ts index d975ea872d1b..51858d0efabc 100644 --- a/packages/svelte/src/reactivity/url.test.ts +++ b/packages/svelte/src/reactivity/url.test.ts @@ -130,6 +130,7 @@ test('url fine grained tests', () => { toJSON: true, toString: true }; + let test_description: string = ''; const reset_change = () => { for (const key of Object.keys(changes) as Array) { @@ -140,93 +141,110 @@ test('url fine grained tests', () => { const cleanup = effect_root(() => { render_effect(() => { url.hash; - assert.equal(changes.hash, true); + assert.equal(changes.hash, true, test_description); }); render_effect(() => { url.host; - assert.equal(changes.host, true); + assert.equal(changes.host, true, test_description); }); render_effect(() => { url.hostname; - assert.equal(changes.hostname, true); + assert.equal(changes.hostname, true, test_description); }); render_effect(() => { url.href; - assert.equal(changes.href, true); + assert.equal(changes.href, true, test_description); }); render_effect(() => { url.origin; - assert.equal(changes.origin, true); + assert.equal(changes.origin, true, test_description); }); render_effect(() => { url.search; - assert.equal(changes.search, true); + assert.equal(changes.search, true, test_description); }); render_effect(() => { url.searchParams.get('fohoov'); - assert.equal(changes.searchParams, true); - }); - - render_effect(() => { - url; - reset_change(); + assert.equal(changes.searchParams, true, test_description); }); }); flushSync(() => { + reset_change(); changes = { ...changes, origin: true, host: true, pathname: true, href: true }; + test_description = 'changing href'; url.href = 'https://www.google.com/test'; }); flushSync(() => { + reset_change(); changes = { ...changes, origin: true, href: true }; + test_description = 'changing protocol'; url.protocol = 'http'; }); flushSync(() => { + reset_change(); + test_description = 'changing protocol to same thing'; url.protocol = 'http'; }); flushSync(() => { + reset_change(); changes = { ...changes, hash: true, href: true }; + test_description = 'changing hash'; url.hash = 'new-hash'; }); flushSync(() => { + reset_change(); + test_description = 'changing hash to same thing'; url.hash = 'new-hash'; }); flushSync(() => { - changes = { ...changes, hostname: true, href: true }; + reset_change(); + changes = { ...changes, hostname: true, host: true, href: true }; + test_description = 'changing hostname'; url.hostname = 'fohoov'; }); flushSync(() => { - changes = { ...changes, search: true, searchParams: true }; + reset_change(); + changes = { ...changes, href: true, search: true, searchParams: true }; + test_description = 'changing search'; url.search = 'fohoov=true'; }); flushSync(() => { + reset_change(); + test_description = 'changing search to same thing'; url.search = 'fohoov=true'; }); flushSync(() => { - changes = { ...changes, search: true, searchParams: true }; + reset_change(); + changes = { ...changes, href: true, search: true, searchParams: true }; + test_description = 'changing search param to false'; url.search = 'fohoov=false'; }); flushSync(() => { - changes = { ...changes, search: true, searchParams: true }; + reset_change(); + changes = { ...changes, href: true, search: true, searchParams: true }; + test_description = 'clearing search'; url.search = ''; }); flushSync(() => { + reset_change(); + test_description = 'clearing search again (expects no change)'; url.search = ''; }); From 11f2508eb09df4744c3d9684c7a953b4233ac0fa Mon Sep 17 00:00:00 2001 From: godzylinux Date: Fri, 24 May 2024 23:45:06 +0330 Subject: [PATCH 64/71] improved date tests --- packages/svelte/src/reactivity/date.test.ts | 88 ++++++++++++++++++++- 1 file changed, 87 insertions(+), 1 deletion(-) diff --git a/packages/svelte/src/reactivity/date.test.ts b/packages/svelte/src/reactivity/date.test.ts index 30905f542dc5..7c868dde4635 100644 --- a/packages/svelte/src/reactivity/date.test.ts +++ b/packages/svelte/src/reactivity/date.test.ts @@ -551,7 +551,7 @@ test('date.setSeconds - edge cases', () => { cleanup(); }); -test('ReactiveDate propagated changes', () => { +test('Date propagated changes', () => { const date = new ReactiveDate(initial_date); const log: any = []; @@ -582,6 +582,92 @@ test('ReactiveDate propagated changes', () => { cleanup(); }); +test('date fine grained tests', () => { + const date = new ReactiveDate(initial_date); + + let changes: Record = { + getFullYear: true, + getUTCFullYear: true, + getMonth: true, + getUTCMonth: true, + getDate: true, + getUTCDate: true, + getDay: true, + getUTCDay: true, + getHours: true, + getUTCHours: true, + getMinutes: true, + getUTCMinutes: true, + getMilliseconds: true, + getUTCMilliseconds: true + }; + let test_description: string = ''; + + const reset_change = () => { + for (const key of Object.keys(changes)) { + changes[key] = false; + } + }; + + const cleanup = effect_root(() => { + for (const key of Object.keys(date)) { + render_effect(() => { + date[key](); + assert.equal(changes[key], true, test_description); + }); + } + }); + + flushSync(() => { + reset_change(); + changes = { + ...changes, + getFullYear: true, + getUTCFullYear: true, + getDate: true, + getUTCDate: true + }; + test_description = 'changing full that will cause month change as well'; + date.setFullYear(initial_date.getFullYear() + 1, initial_date.getMonth() + 1); + }); + + flushSync(() => { + reset_change(); + changes = { + ...changes, + getDate: true, + getUTCDate: true, + getDay: true, + getUTCDay: true, + getHours: true, + getUTCHours: true, + getMinutes: true, + getUTCMinutes: true, + getSeconds: true, + getUTCSeconds: true, + getMilliSeconds: true, + getUTCMilliSeconds: true + }; + test_description = 'changing seconds that will change day/hour/minutes/seconds/milliseconds'; + date.setSeconds(60 * 60 * 25, 10); + }); + + flushSync(() => { + reset_change(); + changes = { + ...changes, + getMonth: true, + getUTCMonth: true, + getMilliSeconds: true, + getUTCMilliSeconds: true + }; + test_description = 'changing month'; + date.setMonth(date.getMonth() + 1); + }); + + cleanup(); +}); + test('Date.instanceOf', () => { assert.equal(new ReactiveDate() instanceof Date, true); }); From 65c006490f5f403b50ce39e3c3888011df6e7748 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Sat, 25 May 2024 00:07:58 +0330 Subject: [PATCH 65/71] added comments describing why we use get_registered_params before notifying read methods --- packages/svelte/src/reactivity/map.js | 2 ++ packages/svelte/src/reactivity/set.js | 1 + 2 files changed, 3 insertions(+) diff --git a/packages/svelte/src/reactivity/map.js b/packages/svelte/src/reactivity/map.js index 34ef741ca73f..6d3122eaf151 100644 --- a/packages/svelte/src/reactivity/map.js +++ b/packages/svelte/src/reactivity/map.js @@ -20,6 +20,7 @@ export const ReactiveMap = make_reactive(Map, { } options.get_registered_params('has')?.forEach((value, param) => { + // because we don't want to notify `has` for items that are currently not in the map if (!options.value.has(param)) { return; } @@ -27,6 +28,7 @@ export const ReactiveMap = make_reactive(Map, { }); options.get_registered_params('get')?.forEach((value, param) => { + // because we don't want to notify `get` for items that are currently not in the map if (!options.value.has(param)) { return; } diff --git a/packages/svelte/src/reactivity/set.js b/packages/svelte/src/reactivity/set.js index 42bc572c9d08..9990da9aaf2c 100644 --- a/packages/svelte/src/reactivity/set.js +++ b/packages/svelte/src/reactivity/set.js @@ -16,6 +16,7 @@ export const ReactiveSet = make_reactive(Set, { return false; } options.get_registered_params('has')?.forEach((value, param) => { + // because we don't want to notify `has` for items that are currently not in the set if (!options.value.has(param)) { return; } From c48f6ff03e7330a2409ca08158e0efe7d205c1f4 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Sat, 25 May 2024 00:35:52 +0330 Subject: [PATCH 66/71] improved tests --- packages/svelte/src/reactivity/date.test.ts | 24 +++++++---- packages/svelte/src/reactivity/url.test.ts | 48 ++++++--------------- 2 files changed, 28 insertions(+), 44 deletions(-) diff --git a/packages/svelte/src/reactivity/date.test.ts b/packages/svelte/src/reactivity/date.test.ts index 7c868dde4635..2c095f69b275 100644 --- a/packages/svelte/src/reactivity/date.test.ts +++ b/packages/svelte/src/reactivity/date.test.ts @@ -610,10 +610,10 @@ test('date fine grained tests', () => { }; const cleanup = effect_root(() => { - for (const key of Object.keys(date)) { + for (const key of Object.keys(changes)) { render_effect(() => { date[key](); - assert.equal(changes[key], true, test_description); + assert.equal(changes[key], true, `${test_description}: for ${key}`); }); } }); @@ -625,9 +625,13 @@ test('date fine grained tests', () => { getFullYear: true, getUTCFullYear: true, getDate: true, - getUTCDate: true + getUTCDate: true, + getMonth: true, + getUTCMonth: true, + getDay: true, + getUTCDay: true }; - test_description = 'changing full that will cause month change as well'; + test_description = 'changing setFullYear that will cause month/day change as well'; date.setFullYear(initial_date.getFullYear() + 1, initial_date.getMonth() + 1); }); @@ -645,11 +649,11 @@ test('date fine grained tests', () => { getUTCMinutes: true, getSeconds: true, getUTCSeconds: true, - getMilliSeconds: true, - getUTCMilliSeconds: true + getMilliseconds: true, + getUTCMilliseconds: true }; test_description = 'changing seconds that will change day/hour/minutes/seconds/milliseconds'; - date.setSeconds(60 * 60 * 25, 10); + date.setSeconds(60 * 60 * 25 + 1, 10); }); flushSync(() => { @@ -658,8 +662,10 @@ test('date fine grained tests', () => { ...changes, getMonth: true, getUTCMonth: true, - getMilliSeconds: true, - getUTCMilliSeconds: true + getDay: true, + getUTCDay: true, + getMilliseconds: true, + getUTCMilliseconds: true }; test_description = 'changing month'; date.setMonth(date.getMonth() + 1); diff --git a/packages/svelte/src/reactivity/url.test.ts b/packages/svelte/src/reactivity/url.test.ts index 51858d0efabc..ac5026fc2834 100644 --- a/packages/svelte/src/reactivity/url.test.ts +++ b/packages/svelte/src/reactivity/url.test.ts @@ -114,7 +114,7 @@ test('url.href', () => { test('url fine grained tests', () => { const url = new ReactiveURL('https://svelte.dev/'); - let changes: Record = { + let changes: Record = { hash: true, host: true, hostname: true, @@ -126,9 +126,7 @@ test('url fine grained tests', () => { port: true, protocol: true, search: true, - searchParams: true, - toJSON: true, - toString: true + searchParams: true }; let test_description: string = ''; @@ -139,39 +137,19 @@ test('url fine grained tests', () => { }; const cleanup = effect_root(() => { - render_effect(() => { - url.hash; - assert.equal(changes.hash, true, test_description); - }); - - render_effect(() => { - url.host; - assert.equal(changes.host, true, test_description); - }); - - render_effect(() => { - url.hostname; - assert.equal(changes.hostname, true, test_description); - }); - - render_effect(() => { - url.href; - assert.equal(changes.href, true, test_description); - }); - - render_effect(() => { - url.origin; - assert.equal(changes.origin, true, test_description); - }); - - render_effect(() => { - url.search; - assert.equal(changes.search, true, test_description); - }); + for (const key of Object.keys(changes) as Array) { + if (key === 'searchParams') { + continue; + } + render_effect(() => { + url[key]; + assert.equal(changes[key], true, `${test_description}: for ${key}`); + }); + } render_effect(() => { url.searchParams.get('fohoov'); - assert.equal(changes.searchParams, true, test_description); + assert.equal(changes.searchParams, true, `${test_description}: for searchParams`); }); }); @@ -184,7 +162,7 @@ test('url fine grained tests', () => { flushSync(() => { reset_change(); - changes = { ...changes, origin: true, href: true }; + changes = { ...changes, protocol: true, origin: true, href: true }; test_description = 'changing protocol'; url.protocol = 'http'; }); From cf2f76ff77b4e449109d496cf6e8da3b14db281f Mon Sep 17 00:00:00 2001 From: godzylinux Date: Sat, 25 May 2024 01:03:57 +0330 Subject: [PATCH 67/71] moved `modified_date_to_compare` out of function to prevent creating a new instance on each change --- packages/svelte/src/reactivity/date.js | 25 +++++++++++---------- packages/svelte/src/reactivity/date.test.ts | 1 + 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/svelte/src/reactivity/date.js b/packages/svelte/src/reactivity/date.js index f9305157af66..dd07db31047b 100644 --- a/packages/svelte/src/reactivity/date.js +++ b/packages/svelte/src/reactivity/date.js @@ -1,5 +1,6 @@ import { make_reactive } from './utils.js'; +const modified_date_to_compare = new Date(); /** * we have to create a new Date to compare, because setting `X` might or might not affect `Y` * for instance calling `date.setMonth(55)` will also change the `date.getYear()` @@ -12,57 +13,57 @@ import { make_reactive } from './utils.js'; * @return {boolean} - returns true if any changes happened */ const notify_datetime_changes = (options, ...params) => { + modified_date_to_compare.setTime(options.value.getTime()); + let is_time_changed = false; let is_date_changed = false; - const new_datetime = new Date(options.value); - // @ts-ignore - new_datetime[options.property](...params); + modified_date_to_compare[options.property](...params); - if (options.value.getFullYear() !== new_datetime.getFullYear()) { + if (options.value.getFullYear() !== modified_date_to_compare.getFullYear()) { options.notify_read_properties(['getFullYear', 'getUTCFullYear']); is_date_changed = true; } // @ts-expect-error - if (options.value.getYear && options.value.getYear() !== new_datetime.getYear()) { + if (options.value.getYear && options.value.getYear() !== modified_date_to_compare.getYear()) { // @ts-expect-error options.notify_read_properties(['getYear']); is_date_changed = true; } - if (options.value.getMonth() !== new_datetime.getMonth()) { + if (options.value.getMonth() !== modified_date_to_compare.getMonth()) { options.notify_read_properties(['getMonth', 'getUTCMonth']); is_date_changed = true; } - if (options.value.getDate() !== new_datetime.getDate()) { + if (options.value.getDate() !== modified_date_to_compare.getDate()) { options.notify_read_properties(['getDate', 'getUTCDate']); is_date_changed = true; } - if (options.value.getDay() !== new_datetime.getDay()) { + if (options.value.getDay() !== modified_date_to_compare.getDay()) { options.notify_read_properties(['getDay', 'getUTCDay']); is_date_changed = true; } - if (options.value.getHours() !== new_datetime.getHours()) { + if (options.value.getHours() !== modified_date_to_compare.getHours()) { options.notify_read_properties(['getHours', 'getUTCHours']); is_time_changed = true; } - if (options.value.getMinutes() !== new_datetime.getMinutes()) { + if (options.value.getMinutes() !== modified_date_to_compare.getMinutes()) { options.notify_read_properties(['getMinutes', 'getUTCMinutes']); is_time_changed = true; } - if (options.value.getSeconds() !== new_datetime.getSeconds()) { + if (options.value.getSeconds() !== modified_date_to_compare.getSeconds()) { options.notify_read_properties(['getSeconds', 'getUTCSeconds']); is_time_changed = true; } - if (options.value.getMilliseconds() !== new_datetime.getMilliseconds()) { + if (options.value.getMilliseconds() !== modified_date_to_compare.getMilliseconds()) { options.notify_read_properties(['getMilliseconds', 'getUTCMilliseconds']); is_time_changed = true; } diff --git a/packages/svelte/src/reactivity/date.test.ts b/packages/svelte/src/reactivity/date.test.ts index 2c095f69b275..0146d9ac3ba8 100644 --- a/packages/svelte/src/reactivity/date.test.ts +++ b/packages/svelte/src/reactivity/date.test.ts @@ -612,6 +612,7 @@ test('date fine grained tests', () => { const cleanup = effect_root(() => { for (const key of Object.keys(changes)) { render_effect(() => { + // @ts-ignore date[key](); assert.equal(changes[key], true, `${test_description}: for ${key}`); }); From d59a51a796f37aeac825112476db408cbce64633 Mon Sep 17 00:00:00 2001 From: godzylinux Date: Sat, 25 May 2024 20:37:22 +0330 Subject: [PATCH 68/71] simplified interceptors for date --- packages/svelte/src/reactivity/date.js | 60 +++++++------------------- 1 file changed, 15 insertions(+), 45 deletions(-) diff --git a/packages/svelte/src/reactivity/date.js b/packages/svelte/src/reactivity/date.js index dd07db31047b..7259f8b6e17a 100644 --- a/packages/svelte/src/reactivity/date.js +++ b/packages/svelte/src/reactivity/date.js @@ -125,51 +125,21 @@ export const ReactiveDate = make_reactive(Date, { 'toLocaleTimeString' ], interceptors: { - setDate: (options, ...params) => { - return notify_datetime_changes(options, ...params); - }, - setFullYear: (options, ...params) => { - return notify_datetime_changes(options, ...params); - }, - setHours: (options, ...params) => { - return notify_datetime_changes(options, ...params); - }, - setMilliseconds: (options, ...params) => { - return notify_datetime_changes(options, ...params); - }, - setMinutes: (options, ...params) => { - return notify_datetime_changes(options, ...params); - }, - setMonth: (options, ...params) => { - return notify_datetime_changes(options, ...params); - }, - setSeconds: (options, ...params) => { - return notify_datetime_changes(options, ...params); - }, - setTime: (options, ...params) => { - return notify_datetime_changes(options, ...params); - }, - setUTCDate: (options, ...params) => { - return notify_datetime_changes(options, ...params); - }, - setUTCFullYear: (options, ...params) => { - return notify_datetime_changes(options, ...params); - }, - setUTCHours: (options, ...params) => { - return notify_datetime_changes(options, ...params); - }, - setUTCMilliseconds: (options, ...params) => { - return notify_datetime_changes(options, ...params); - }, - setUTCMinutes: (options, ...params) => { - return notify_datetime_changes(options, ...params); - }, - setUTCMonth: (options, ...params) => { - return notify_datetime_changes(options, ...params); - }, - setUTCSeconds: (options, ...params) => { - return notify_datetime_changes(options, ...params); - }, + setDate: (options, ...params) => notify_datetime_changes(options, ...params), + setFullYear: (options, ...params) => notify_datetime_changes(options, ...params), + setHours: (options, ...params) => notify_datetime_changes(options, ...params), + setMilliseconds: (options, ...params) => notify_datetime_changes(options, ...params), + setMinutes: (options, ...params) => notify_datetime_changes(options, ...params), + setMonth: (options, ...params) => notify_datetime_changes(options, ...params), + setSeconds: (options, ...params) => notify_datetime_changes(options, ...params), + setTime: (options, ...params) => notify_datetime_changes(options, ...params), + setUTCDate: (options, ...params) => notify_datetime_changes(options, ...params), + setUTCFullYear: (options, ...params) => notify_datetime_changes(options, ...params), + setUTCHours: (options, ...params) => notify_datetime_changes(options, ...params), + setUTCMilliseconds: (options, ...params) => notify_datetime_changes(options, ...params), + setUTCMinutes: (options, ...params) => notify_datetime_changes(options, ...params), + setUTCMonth: (options, ...params) => notify_datetime_changes(options, ...params), + setUTCSeconds: (options, ...params) => notify_datetime_changes(options, ...params), // @ts-expect-error - deprecated method setYear: (options, ...params) => { // it might be removed from browsers From 474155b8ff681bf8af09da8907cb5c58fece8671 Mon Sep 17 00:00:00 2001 From: FoHoOV <53280503+FoHoOV@users.noreply.github.com> Date: Thu, 30 May 2024 15:46:11 +0330 Subject: [PATCH 69/71] improved date tests --- packages/svelte/src/reactivity/date.test.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/svelte/src/reactivity/date.test.ts b/packages/svelte/src/reactivity/date.test.ts index 0146d9ac3ba8..90ab25e73fe2 100644 --- a/packages/svelte/src/reactivity/date.test.ts +++ b/packages/svelte/src/reactivity/date.test.ts @@ -598,14 +598,16 @@ test('date fine grained tests', () => { getUTCHours: true, getMinutes: true, getUTCMinutes: true, + getSeconds: true, + getUTCSeconds: true, getMilliseconds: true, getUTCMilliseconds: true }; let test_description: string = ''; - const reset_change = () => { - for (const key of Object.keys(changes)) { - changes[key] = false; + const expect_all_changes_to_be_false = () => { + for (const key of Object.keys(changes) as Array) { + assert.equal(changes[key], false, `${test_description}: effect for ${key} was not fired`); } }; @@ -615,18 +617,17 @@ test('date fine grained tests', () => { // @ts-ignore date[key](); assert.equal(changes[key], true, `${test_description}: for ${key}`); + changes[key] = false; }); } }); flushSync(() => { - reset_change(); + expect_all_changes_to_be_false(); changes = { ...changes, getFullYear: true, getUTCFullYear: true, - getDate: true, - getUTCDate: true, getMonth: true, getUTCMonth: true, getDay: true, @@ -637,7 +638,7 @@ test('date fine grained tests', () => { }); flushSync(() => { - reset_change(); + expect_all_changes_to_be_false(); changes = { ...changes, getDate: true, @@ -654,11 +655,11 @@ test('date fine grained tests', () => { getUTCMilliseconds: true }; test_description = 'changing seconds that will change day/hour/minutes/seconds/milliseconds'; - date.setSeconds(60 * 60 * 25 + 1, 10); + date.setSeconds(61 * 60 * 25 + 1, 10); }); flushSync(() => { - reset_change(); + expect_all_changes_to_be_false(); changes = { ...changes, getMonth: true, From 245946171fa3fed29cdd0d75b86a01c3951b1954 Mon Sep 17 00:00:00 2001 From: FoHoOV <53280503+FoHoOV@users.noreply.github.com> Date: Thu, 30 May 2024 15:46:25 +0330 Subject: [PATCH 70/71] improved reactivity for search-params --- .../src/reactivity/url-search-params.js | 19 +++++++++++++++---- .../src/reactivity/url-search-params.test.ts | 9 ++++++++- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/reactivity/url-search-params.js b/packages/svelte/src/reactivity/url-search-params.js index bb17192365ad..2d56f85f5eab 100644 --- a/packages/svelte/src/reactivity/url-search-params.js +++ b/packages/svelte/src/reactivity/url-search-params.js @@ -5,22 +5,33 @@ export const ReactiveURLSearchParams = make_reactive(URLSearchParams, { read_properties: ['get', 'has', 'getAll'], interceptors: { set: (options, ...params) => { - if (typeof params[0] == 'string' && options.value.get(params[0]) === params[1]) { + const value = options.value.get(/**@type {string} */ (params[0])); + const value_has_changed = value !== /**@type {string} */ (params[1]).toString(); + if (value && !value_has_changed) { return false; } - options.notify_read_properties(['get', 'has', 'getAll'], params[0]); + + if (!value) { + options.notify_read_properties(['has'], params[0]); + } + + if (value_has_changed) { + options.notify_read_properties(['get'], params[0]); + } + + options.notify_read_properties(['getAll'], params[0]); return true; }, append: (options, ...params) => { options.notify_read_properties(['getAll'], params[0]); - if (typeof params[0] == 'string' && !options.value.has(params[0])) { + if (!options.value.has(/**@type {string} */ (params[0]))) { options.notify_read_properties(['get', 'has'], params[0]); } return true; }, delete: (options, ...params) => { - if (typeof params[0] == 'string' && !options.value.has(params[0])) { + if (!options.value.has(/**@type {string} */ (params[0]))) { return false; } options.notify_read_properties(['get', 'has', 'getAll'], params[0]); diff --git a/packages/svelte/src/reactivity/url-search-params.test.ts b/packages/svelte/src/reactivity/url-search-params.test.ts index 6673710fc169..207241a2acf9 100644 --- a/packages/svelte/src/reactivity/url-search-params.test.ts +++ b/packages/svelte/src/reactivity/url-search-params.test.ts @@ -98,6 +98,9 @@ test('URLSearchParams.get', () => { render_effect(() => { log.push(params.get('c')); }); + render_effect(() => { + log.push(params.get('e')); + }); }); flushSync(() => { @@ -112,7 +115,11 @@ test('URLSearchParams.get', () => { params.delete('a'); }); - assert.deepEqual(log, ['b', 'd', 'new-b', null]); + flushSync(() => { + params.set('q', 'f'); + }); + + assert.deepEqual(log, ['b', 'd', null, 'new-b', null]); cleanup(); }); From d3fd750491ae48a5f4fc3fde8467d702fed0e5ad Mon Sep 17 00:00:00 2001 From: FoHoOV <53280503+FoHoOV@users.noreply.github.com> Date: Sat, 1 Jun 2024 19:07:39 +0330 Subject: [PATCH 71/71] fixed url to work with node18 --- .../src/reactivity/url-search-params.js | 2 + packages/svelte/src/reactivity/url.js | 74 +++++++++---------- packages/svelte/src/reactivity/url.test.ts | 40 +++++----- packages/svelte/src/reactivity/utils.js | 7 +- 4 files changed, 67 insertions(+), 56 deletions(-) diff --git a/packages/svelte/src/reactivity/url-search-params.js b/packages/svelte/src/reactivity/url-search-params.js index 2d56f85f5eab..2beb88f578a1 100644 --- a/packages/svelte/src/reactivity/url-search-params.js +++ b/packages/svelte/src/reactivity/url-search-params.js @@ -7,6 +7,7 @@ export const ReactiveURLSearchParams = make_reactive(URLSearchParams, { set: (options, ...params) => { const value = options.value.get(/**@type {string} */ (params[0])); const value_has_changed = value !== /**@type {string} */ (params[1]).toString(); + if (value && !value_has_changed) { return false; } @@ -20,6 +21,7 @@ export const ReactiveURLSearchParams = make_reactive(URLSearchParams, { } options.notify_read_properties(['getAll'], params[0]); + return true; }, append: (options, ...params) => { diff --git a/packages/svelte/src/reactivity/url.js b/packages/svelte/src/reactivity/url.js index bcff6e24476b..6b483d7d18f9 100644 --- a/packages/svelte/src/reactivity/url.js +++ b/packages/svelte/src/reactivity/url.js @@ -2,24 +2,26 @@ import { untrack } from '../index-client.js'; import { ReactiveURLSearchParams } from './url-search-params.js'; import { make_reactive } from './utils.js'; -// // had to create a subclass for URLWithReactiveSearchParams // because we cannot change the internal `searchParams` reference (which links to the web api implementation) so it requires // some custom logic -// class URLWithReactiveSearchParams extends URL { /** * @type {InstanceType} */ #reactive_search_params; + /** + * @type {boolean} + */ + #is_in_middle_of_update = false; + /** * @param {ConstructorParameters} params */ constructor(...params) { super(...params); - - this.#reactive_search_params = new ReactiveURLSearchParams(super.searchParams); + this.#reactive_search_params = new ReactiveURLSearchParams(super.search); } /** @@ -34,7 +36,7 @@ class URLWithReactiveSearchParams extends URL { */ get search() { this.searchParams.toString(); - this.#sync_params_with_url('search_params'); + this.#sync_params_with_url_if_not_blocked(); return super.search; } @@ -42,8 +44,10 @@ class URLWithReactiveSearchParams extends URL { * @override */ set search(value) { + this.#is_in_middle_of_update = true; super.search = value; this.#sync_params_with_url('url'); + this.#is_in_middle_of_update = false; } /** @@ -51,7 +55,7 @@ class URLWithReactiveSearchParams extends URL { */ get href() { this.searchParams.toString(); - this.#sync_params_with_url('search_params'); + this.#sync_params_with_url_if_not_blocked(); return super.href; } @@ -59,63 +63,60 @@ class URLWithReactiveSearchParams extends URL { * @override */ set href(value) { + this.#is_in_middle_of_update = true; super.href = value; this.#sync_params_with_url('url'); + this.#is_in_middle_of_update = false; } /** * @param {"url" | "search_params"} changed_value */ #sync_params_with_url(changed_value) { - if (super.searchParams.toString() === this.searchParams.toString()) { - return; - } - - if (changed_value == 'url') { - this.#update_search_params_from_url(); - } else { - // updating url from params - this.search = this.searchParams.toString(); - } + untrack(() => { + if ( + super.search.length === 0 + ? this.searchParams.size === 0 + : super.search === `?${this.searchParams}` + ) { + return; + } + + if (changed_value == 'url') { + this.#update_search_params_from_url(); + } else { + // updating url from params + this.search = this.searchParams.toString(); + } + }); } #update_search_params_from_url() { - /** - * keeping track of this is required because we have to keep the order in which they are updated - * @type {string[]} - */ - const keys_with_no_change = []; - // remove keys that don't exist anymore and notify others for (const [key, value] of Array.from(this.searchParams.entries())) { - if (!super.searchParams.has(key) || value == super.searchParams.get(key)) { - keys_with_no_change.push(key); - untrack(() => { - this.searchParams.delete(key); - }); - continue; - } this.searchParams.delete(key); } // set or update keys based on the params for (const [key, value] of super.searchParams.entries()) { - if (keys_with_no_change.includes(key)) { - untrack(() => { - this.searchParams.set(key, value); - }); - continue; - } this.searchParams.set(key, value); } } + #sync_params_with_url_if_not_blocked() { + if (!this.#is_in_middle_of_update) { + this.#is_in_middle_of_update = true; + this.#sync_params_with_url('search_params'); + this.#is_in_middle_of_update = false; + } + } + /** * @override */ toString() { this.searchParams.toString(); - this.#sync_params_with_url('search_params'); + this.#sync_params_with_url_if_not_blocked(); return super.toString(); } } @@ -201,7 +202,6 @@ export const ReactiveURL = make_reactive(URLWithReactiveSearchParams, { options.notify_read_properties(['href', 'origin', 'host', 'port']); return true; }, - pathname: (options, ...params) => { if (options.value.pathname === add_character_if_not_exists(params[0], '/', 'prepend')) { return false; diff --git a/packages/svelte/src/reactivity/url.test.ts b/packages/svelte/src/reactivity/url.test.ts index ac5026fc2834..f94963019ed7 100644 --- a/packages/svelte/src/reactivity/url.test.ts +++ b/packages/svelte/src/reactivity/url.test.ts @@ -130,9 +130,13 @@ test('url fine grained tests', () => { }; let test_description: string = ''; - const reset_change = () => { + const expect_all_changes_to_be_false = () => { for (const key of Object.keys(changes) as Array) { - changes[key] = false; + assert.equal( + changes[key], + false, + `${test_description}: effect for ${key} was not fired (value=${url[key]})` + ); } }; @@ -142,86 +146,88 @@ test('url fine grained tests', () => { continue; } render_effect(() => { - url[key]; - assert.equal(changes[key], true, `${test_description}: for ${key}`); + const value = url[key]; + assert.equal(changes[key], true, `${test_description}: for ${key} (value=${value})`); + changes[key] = false; }); } render_effect(() => { url.searchParams.get('fohoov'); - assert.equal(changes.searchParams, true, `${test_description}: for searchParams`); + assert.equal(changes['searchParams'], true, `${test_description}: for searchParams`); + changes['searchParams'] = false; }); }); flushSync(() => { - reset_change(); + expect_all_changes_to_be_false(); changes = { ...changes, origin: true, host: true, pathname: true, href: true }; test_description = 'changing href'; url.href = 'https://www.google.com/test'; }); flushSync(() => { - reset_change(); + expect_all_changes_to_be_false(); changes = { ...changes, protocol: true, origin: true, href: true }; test_description = 'changing protocol'; url.protocol = 'http'; }); flushSync(() => { - reset_change(); + expect_all_changes_to_be_false(); test_description = 'changing protocol to same thing'; url.protocol = 'http'; }); flushSync(() => { - reset_change(); + expect_all_changes_to_be_false(); changes = { ...changes, hash: true, href: true }; test_description = 'changing hash'; url.hash = 'new-hash'; }); flushSync(() => { - reset_change(); + expect_all_changes_to_be_false(); test_description = 'changing hash to same thing'; url.hash = 'new-hash'; }); flushSync(() => { - reset_change(); + expect_all_changes_to_be_false(); changes = { ...changes, hostname: true, host: true, href: true }; test_description = 'changing hostname'; url.hostname = 'fohoov'; }); flushSync(() => { - reset_change(); + expect_all_changes_to_be_false(); changes = { ...changes, href: true, search: true, searchParams: true }; test_description = 'changing search'; url.search = 'fohoov=true'; }); flushSync(() => { - reset_change(); + expect_all_changes_to_be_false(); test_description = 'changing search to same thing'; url.search = 'fohoov=true'; }); flushSync(() => { - reset_change(); + expect_all_changes_to_be_false(); changes = { ...changes, href: true, search: true, searchParams: true }; - test_description = 'changing search param to false'; + test_description = 'changing search param fohoov to false'; url.search = 'fohoov=false'; }); flushSync(() => { - reset_change(); + expect_all_changes_to_be_false(); changes = { ...changes, href: true, search: true, searchParams: true }; test_description = 'clearing search'; url.search = ''; }); flushSync(() => { - reset_change(); + expect_all_changes_to_be_false(); test_description = 'clearing search again (expects no change)'; url.search = ''; }); diff --git a/packages/svelte/src/reactivity/utils.js b/packages/svelte/src/reactivity/utils.js index 6875b8d53db6..e60f9c4fe6f9 100644 --- a/packages/svelte/src/reactivity/utils.js +++ b/packages/svelte/src/reactivity/utils.js @@ -282,7 +282,10 @@ function notify_read_properties( } else { (params.length == 0 ? [null] : params).forEach((param) => { const sig = get_signal_for_function(read_methods_signals, name, param); - sig && increment_signal(sig); + if (sig) { + // I did want to use short-circuit like sig && increment_signal(sig) but linter didn't allow me to :( + increment_signal(sig); + } increment_version_signal(initial_version_signal_v, version_signal); }); } @@ -333,7 +336,7 @@ function get_signal_for_function( * @param {import("#client").Source} signal */ function increment_signal(signal) { - signal && set(signal, !signal.v); + set(signal, !signal.v); } /**