Address PR review feedback: restructure tests and simplify asap usage

Author: TudorGR

src/sinks/class-sink.test.ts

@@ -30,81 +30,92 @@ describe('Class Sink', () => {
 
 	describe('Given a class object', () => {
 
-		it('sets classes for truthy attributes on sink', () => {
-			const el = MockElement();
-			const sink = ClassObjectSink(<HTMLElement>el);
+		describe('when a property of the object is a present value', () => {
+
+			it('sets classes for truthy attributes on sink', () => {
+				const el = MockElement();
+				const sink = ClassObjectSink(<HTMLElement>el);
+
+				sink({
+					class1: true,
+					class2: 1,
+					class3: 'yes!',
+				});
+				expect(el.className).toContain('class1');
+				expect(el.className).toContain('class2');
+				expect(el.className).toContain('class3');
+			});
 
-			sink({
-				class1: true,
-				class2: 1,
-				class3: 'yes!',
+			it('clears classes for falsy attributes on sink', () => {
+				const el = MockElement({ className: 'class1 class2 class3' });
+				const sink = ClassObjectSink(<HTMLElement>el);
+				expect(el.className).toContain('class1');
+				expect(el.className).toContain('class2');
+				expect(el.className).toContain('class3');
+
+				sink({
+					class1: false,
+					class2: 0,
+					class3: '',
+				});
+				expect(el.className).not.toContain('class1');
+				expect(el.className).not.toContain('class2');
+				expect(el.className).not.toContain('class3');
 			});
-			expect(el.className).toContain('class1');
-			expect(el.className).toContain('class2');
-			expect(el.className).toContain('class3');
-		});
 
-		it('clears classes for falsy attributes on sink', () => {
-			const el = MockElement({ className: 'class1 class2 class3' });
-			const sink = ClassObjectSink(<HTMLElement>el);
-			expect(el.className).toContain('class1');
-			expect(el.className).toContain('class2');
-			expect(el.className).toContain('class3');
+			it('should not add class when value is false', () => {
+				const el = MockElement();
+				const sink = ClassObjectSink(<HTMLElement>el);
 
-			sink({
-				class1: false,
-				class2: 0,
-				class3: '',
+				sink({
+					dotted: false,
+				});
+
+				expect(el.className).not.toContain('dotted');
+				expect(el.className).toEqual('');
 			});
-			expect(el.className).not.toContain('class1');
-			expect(el.className).not.toContain('class2');
-			expect(el.className).not.toContain('class3');
+
 		});
 
-		it('should not add class when value is false (promise resolved to false)', () => {
-			const el = MockElement();
-			const sink = ClassObjectSink(<HTMLElement>el);
+		describe('when a property of the object is a future value', () => {
 
-			// Simulating a promise that resolved to false
-			sink({
-				dotted: false,
-			});
+			describe('when it resolves/emits false', () => {
 
-			expect(el.className).not.toContain('dotted');
-			expect(el.className).toEqual('');
-		});
+				it('should not add a class corresponding to the property name', async () => {
+					const el = MockElement();
+					const sink = ClassObjectSink(<HTMLElement>el);
 
-		it('should not add class when promise resolves to false', async () => {
-			const el = MockElement();
-			const sink = ClassObjectSink(<HTMLElement>el);
+					const dottedPromise = Promise.resolve(false);
+					sink({
+						dotted: dottedPromise,
+					});
+
+					await dottedPromise;
+
+					expect(el.className).not.toContain('dotted');
+					expect(el.className).toEqual('');
+				});
 
-			// Pass a promise that resolves to false
-			const dottedPromise = Promise.resolve(false);
-			sink({
-				dotted: dottedPromise,
 			});
 
-			// Wait for the promise to resolve
-			await dottedPromise;
+			describe('when it resolves/emits true', () => {
 
-			expect(el.className).not.toContain('dotted');
-			expect(el.className).toEqual('');
-		});
+				it('should add a class corresponding to the property name', async () => {
+					const el = MockElement();
+					const sink = ClassObjectSink(<HTMLElement>el);
 
-		it('should add class when promise resolves to true', async () => {
-			const el = MockElement();
-			const sink = ClassObjectSink(<HTMLElement>el);
+					const activePromise = Promise.resolve(true);
+					sink({
+						active: activePromise,
+					});
 
-			// Pass a promise that resolves to true
-			const activePromise = Promise.resolve(true);
-			sink({
-				active: activePromise,
-			});
+					await activePromise;
 
-			// Wait for the promise to resolve
-			await activePromise;
+					expect(el.className).toContain('active');
+				});
+
+			});
 
-			expect(el.className).toContain('active');
 		});
 
 	});

src/sinks/class-sink.ts

@@ -4,7 +4,6 @@ import type { Sink, ExplicitSink } from "../types/sink";
 
 import { SINK_TAG } from "../constants";
 import { asap } from "../lib/drain";
-import { isFuture } from "../types/futures";
 
 export const TOGGLE_CLASS_SINK_TAG = 'ToggleClass';
 export const CLASS_SINK_TAG = 'class'; // Keeping it same as 'class" attribute for now. Don't change yet...
@@ -37,13 +36,10 @@ export const ClassObjectSink: Sink<Element> = (node: Element) => {
 			: (<(ClassName | ClassRecord)[]>[]).concat(name).forEach(obj => Object.entries(obj)
 					// TODO: support 3-state with toggle
 					.forEach(([k, v]) => {
-						// If v is a future (Promise or Observable), defer the decision
-						// until it resolves, otherwise check immediately
-						if (isFuture(v)) {
-							asap((resolvedValue: any) => resolvedValue ? add(k) : remove(k), v);
-						} else {
-							asap(v ? add : remove, k);
-						}
+						// Use asap to handle both present and future values
+						// For futures (Promise/Observable), it will wait for resolution
+						// For present values, it will execute immediately
+						asap((resolvedValue: any) => resolvedValue ? add(k) : remove(k), v);
 					})
 				)
 	};