Fixed a race condition when removing multiple groups at once

Author: Pckool

.trunk/trunk.yaml

@@ -1,10 +1,10 @@
 version: 0.1
 cli:
-  version: 1.14.0
+  version: 1.15.0
 plugins:
   sources:
     - id: trunk
-      ref: v1.1.0
+      ref: v1.2.1
       uri: https://github.com/trunk-io/plugins
 lint:
   threshold:

packages/plexus-core/src/collection/collection.ts

@@ -105,7 +105,7 @@ interface PlexusCollectionStore<DataType extends Record<string, any>> {
 export type PlexusCollectionInstance<
 	DataType extends Record<string, any> = Record<string, any>,
 	Groups extends GroupMap<DataType> = GroupMap<DataType>,
-	Selectors extends SelectorMap<DataType> = SelectorMap<DataType>,
+	Selectors extends SelectorMap<DataType> = SelectorMap<DataType>
 > = CollectionInstance<DataType, Groups, Selectors>
 /**
  * A Collection Instance
@@ -114,7 +114,7 @@ export type PlexusCollectionInstance<
 export class CollectionInstance<
 	DataTypeInput extends Record<string, any>,
 	Groups extends GroupMap<DataTypeInput>,
-	Selectors extends SelectorMap<DataTypeInput>,
+	Selectors extends SelectorMap<DataTypeInput>
 	// ForeignRefs extends boolean = this['config']['foreignKeys'] extends {} ? true : false
 > {
 	private _internalStore: PlexusCollectionStore<DataTypeInput>
@@ -305,7 +305,7 @@ export class CollectionInstance<
 			return Array.from(addedKeys.values())
 		}
 		const collectFn = (
-			data_: typeof data,
+			dataToCollect: typeof data,
 			groups?: KeyOfMap<Groups>[] | KeyOfMap<Groups>,
 			startedFromInnerBatch?: boolean
 		) => {
@@ -325,13 +325,13 @@ export class CollectionInstance<
 						'debug',
 						`Batched collect call fulfilled for collection ${this.instanceId}`
 					)
-					return collectFn(data_, groups, true)
+					return collectFn(dataToCollect, groups, true)
 				})
 				return this
 			}
 
 			const addedKeys: any[] = collectItems(
-				Array.isArray(data_) ? data_ : [data_]
+				Array.isArray(dataToCollect) ? dataToCollect : [dataToCollect]
 			)
 
 			const defaultGroupName =
@@ -788,10 +788,9 @@ export class CollectionInstance<
 			this._internalStore._data.delete(key)
 		}
 		// if an array, iterate through the keys and remove them each
-		if (Array.isArray(keys)) {
-			keys.forEach(rm)
-		} else {
-			rm(keys)
+		keys = Array.isArray(keys) ? keys : [keys]
+		for (const key of keys) {
+			rm(key)
 		}
 		this.mount()
 		return this
@@ -819,28 +818,29 @@ export class CollectionInstance<
 		keys: DataKey | DataKey[],
 		groups: KeyOfMap<Groups> | KeyOfMap<Groups>[]
 	): this {
+		// normalize
+		keys = Array.isArray(keys) ? keys : [keys]
+		groups = Array.isArray(groups) ? groups : [groups]
+
+		// abort conditions
+		if (!keys.length || !groups.length) return this
+
 		this.mount()
 		const rm = (key) => {
 			key = `${key}`
-			if (Array.isArray(groups)) {
-				for (let groupName of groups) {
-					if (this.isCreatedGroup(groupName)) {
-						this._internalStore._groups.get(groupName)?.remove(key)
-					}
-				}
-			} else if (typeof groups === 'string') {
-				if (this.isCreatedGroup(groups)) {
-					this._internalStore._groups.get(groups)?.remove(key)
+			for (let groupName of groups) {
+				if (this.isCreatedGroup(groupName)) {
+					this._internalStore._groups.get(groupName)?.remove(key)
 				}
 			}
 		}
 
 		// if an array, iterate through the keys and remove them from each associated group
-		if (Array.isArray(keys)) {
-			keys.forEach(rm)
-		} else {
-			rm(keys)
-		}
+		this.instance().runtime.batch(() => {
+			for (let key of keys) {
+				rm(key)
+			}
+		})
 		return this
 		// ! This is commented out because the user may still want to keep the data in the collection. If they want to completely delete the data, they should use `.delete()`
 		// if it's removed from all groups, delete the data entirely
@@ -856,13 +856,9 @@ export class CollectionInstance<
 	clear(groupNames?: KeyOfMap<Groups> | KeyOfMap<Groups>[]): this {
 		// this means we want to clear a group, not the whole collection
 		if (groupNames) {
-			if (Array.isArray(groupNames)) {
-				groupNames.forEach(
-					(groupName) =>
-						this.isCreatedGroup(groupName) && this.getGroup(groupName).clear()
-				)
-			} else {
-				this.isCreatedGroup(groupNames) && this.getGroup(groupNames).clear()
+			groupNames = Array.isArray(groupNames) ? groupNames : [groupNames]
+			for (const groupName of groupNames) {
+				this.isCreatedGroup(groupName) && this.getGroup(groupName).clear()
 			}
 		} else {
 			this.delete(Array.from(this._internalStore._data.keys()))
@@ -1078,7 +1074,7 @@ export class CollectionInstance<
 export function _collection<
 	DataType extends { [key: string]: any },
 	Groups extends GroupMap<DataType> = GroupMap<DataType>,
-	Selectors extends SelectorMap<DataType> = SelectorMap<DataType>,
+	Selectors extends SelectorMap<DataType> = SelectorMap<DataType>
 >(
 	instance: () => PlexusInstance,
 	_config: PlexusCollectionConfig<DataType> = { primaryKey: 'id' } as const

packages/plexus-core/src/collection/data.ts

@@ -25,7 +25,7 @@ interface PlexusDataStore {
 }
 
 export type PlexusDataInstance<
-	DataType extends Record<string, any> = Record<string, any>,
+	DataType extends Record<string, any> = Record<string, any>
 > = CollectionData<DataType>
 export type DataKey = string
 
@@ -38,7 +38,7 @@ type DataObjectType<PK extends string = 'id'> = Record<string, any> & {
  */
 export class CollectionData<
 	DataType extends DataObjectType<PK> = any,
-	PK extends string = string,
+	PK extends string = string
 > extends WatchableMutable<DataType> {
 	private primaryKey: PK
 	readonly key: string

packages/plexus-core/src/computed.ts

@@ -95,7 +95,7 @@ export class ComputedStateInstance<
 
 		this.instance().runtime.log(
 			'info',
-			`Mounting Dependencies (${this.deps
+			`Mounting computed dependencies (${this.deps
 				.map((v) => v?.id ?? 'unknown')
 				.join(', ')}) to Computed state ${this.instanceId}`
 		)

packages/plexus-core/src/instance/engine.ts

@@ -19,13 +19,17 @@ export class EventEngine {
 	 */
 	halt() {
 		this.batching = true
-		return () => {
-			this.batching = false
-			this.pendingEventPayloads.forEach((args, eventId) => {
-				this.emit(eventId, args)
-			})
-			this.pendingEventPayloads.clear()
-		}
+		return () => this.release()
+	}
+	/**
+	 * Emit all stored concatenated events and resume normal event emitting.
+	 */
+	release() {
+		this.batching = false
+		this.pendingEventPayloads.forEach((args, eventId) => {
+			this.emit(eventId, args)
+		})
+		this.pendingEventPayloads.clear()
 	}
 
 	on(eventId: string, listener: PlexusInternalWatcher, origin?: string) {

packages/plexus-core/src/instance/instance.ts

@@ -202,10 +202,7 @@ export function instance(
 			'info',
 			'Instance initialized'
 		)
-	} else if (
-		config &&
-		!isEqual(config, getPlexusInstance(config?.id || '').settings)
-	) {
+	} else if (config) {
 		getPlexusInstance(config?.id || '').settings = deepMerge(
 			getPlexusInstance(config?.id || '').settings,
 			config

tests/basic.test.ts

@@ -67,15 +67,19 @@ describe('State Core Functionality', () => {
 	})
 
 	test('Checking state().set()', () => {
+		objectState.reset()
 		// check .set(value: object)
 		objectState.set({ a: { b: false } })
 		// check if the object is actually merged and children props do get overwritten
 		expect(objectState.value.a?.b).toBe(false)
 	})
 
 	test('Checking state().patch()', () => {
+		objectState.reset()
+		console.log(objectState.value)
 		// can the object deep merge?
 		objectState.patch({ a: { b: false } })
+		console.log(objectState.value)
 		expect(objectState.value.a?.a).toBe(true)
 		// check that other value is still there
 		expect(objectState.value.b).toBe(true)

tests/computed.test.ts

@@ -127,6 +127,7 @@ describe('Testing Computed State Function', () => {
 
 		expect(core.numOfReading.value).toBe(0)
 	})
+	// currently, this test fails because the default group updates BEFORE the custom group updates. This means we need to update the engine to only emit the event after all the groups have been updated
 	test('Computed can watch a default collection group', () => {
 		instance({ logLevel: 'debug' })
 		collections.books.getGroup('READING')?.watch((v) => {
@@ -146,7 +147,17 @@ describe('Testing Computed State Function', () => {
 
 		expect(core.numOfReadingReactiveToAll.value).toBe(1)
 
+		console.log(
+			`(t6sawo4bjhkv47839d3) groups pre-delete: ${core.collections.books.getGroupsOf(
+				't6sawo4bjhkv47839d3'
+			)}`
+		)
 		core.collections.books.delete('t6sawo4bjhkv47839d3')
+		console.log(
+			`(t6sawo4bjhkv47839d3) groups post-delete: ${core.collections.books.getGroupsOf(
+				't6sawo4bjhkv47839d3'
+			)}`
+		)
 		console.log(
 			'numOfReadingReactiveToAll.value',
 			core.numOfReadingReactiveToAll.value,

tests/state.test.ts

@@ -173,4 +173,8 @@ describe('Testing State Function', () => {
 		stateWithFetchFnTest.fetch()
 		expect(stateWithFetchFnTest.value).toBe('a new string!' + 'new value')
 	})
+	test('persist', () => {
+		const value = state(1).persist('test')
+		expect(value.value).toBe(1)
+	})
 })