chore(datastore): clean up code to follow Amplify patterns

- Remove unnecessary comments
- Simplify code structure
- Follow minimal Amplify style
- Clean up test files

Author: anivar

packages/datastore/__tests__/subscription-variables-edge-cases.test.ts

@@ -44,7 +44,6 @@ describe('Subscription Variables - Edge Cases & Safety', () => {
 			const schema = createTestSchema();
 			const sharedObject = { storeId: 'initial' };
 
-			// First call
 			const cache = new WeakMap();
 			const result1 = processSubscriptionVariables(
 				schema.namespaces.user.models.Todo,
@@ -53,29 +52,24 @@ describe('Subscription Variables - Edge Cases & Safety', () => {
 				cache,
 			);
 
-			// Mutate the shared object
 			sharedObject.storeId = 'mutated';
 
-			// Second call - should get cached value, not mutated
 			const result2 = processSubscriptionVariables(
 				schema.namespaces.user.models.Todo,
 				TransformerMutationType.CREATE,
 				sharedObject,
 				cache,
 			);
 
-			// Results should be equal (same cached object)
 			expect(result1).toEqual(result2);
-			// But changing the original shouldn't affect results
 			expect(result2?.storeId).not.toBe('mutated');
 		});
 
 		it('should handle circular references gracefully', () => {
 			const schema = createTestSchema();
 			const circular: any = { storeId: 'test' };
-			circular.self = circular; // Create circular reference
+			circular.self = circular;
 
-			// Should handle circular reference without crashing
 			const cache = new WeakMap();
 			const result = processSubscriptionVariables(
 				schema.namespaces.user.models.Todo,
@@ -84,7 +78,6 @@ describe('Subscription Variables - Edge Cases & Safety', () => {
 				cache,
 			);
 
-			// Should return the object but not cache it (due to JSON.stringify failure)
 			expect(result).toBeDefined();
 			expect(result?.storeId).toBe('test');
 		});
@@ -134,7 +127,6 @@ describe('Subscription Variables - Edge Cases & Safety', () => {
 		it('should handle function that throws', () => {
 			const schema = createTestSchema();
 
-			// Should not crash
 			const cache = new WeakMap();
 			const mockFn = () => {
 				throw new Error('Function error');
@@ -180,7 +172,6 @@ describe('Subscription Variables - Edge Cases & Safety', () => {
 			const schema = createTestSchema();
 			const mockFn = jest.fn(() => ({ storeId: 'test' }));
 
-			// Call multiple times for same operation
 			const cache = new WeakMap();
 			for (let i = 0; i < 5; i++) {
 				processSubscriptionVariables(
@@ -191,19 +182,16 @@ describe('Subscription Variables - Edge Cases & Safety', () => {
 				);
 			}
 
-			// Should only be called once
 			expect(mockFn).toHaveBeenCalledTimes(1);
 			expect(mockFn).toHaveBeenCalledWith(TransformerMutationType.CREATE);
 
-			// Call for different operation
 			processSubscriptionVariables(
 				schema.namespaces.user.models.Todo,
 				TransformerMutationType.UPDATE,
 				mockFn,
 				cache,
 			);
 
-			// Should be called again for new operation
 			expect(mockFn).toHaveBeenCalledTimes(2);
 			expect(mockFn).toHaveBeenCalledWith(TransformerMutationType.UPDATE);
 		});
@@ -226,7 +214,6 @@ describe('Subscription Variables - Edge Cases & Safety', () => {
 				},
 			);
 
-			// First call
 			let cache = new WeakMap();
 			processSubscriptionVariables(
 				schema.namespaces.user.models.Todo,
@@ -236,19 +223,16 @@ describe('Subscription Variables - Edge Cases & Safety', () => {
 			);
 			expect(mockFn).toHaveBeenCalledTimes(1);
 
-			// Stop processor (clears cache) - simulate by creating new cache
 			await processor.stop();
 			cache = new WeakMap();
 
-			// Call again after stop
 			processSubscriptionVariables(
 				schema.namespaces.user.models.Todo,
 				TransformerMutationType.CREATE,
 				mockFn,
 				cache,
 			);
 
-			// Should be called again since cache was cleared
 			expect(mockFn).toHaveBeenCalledTimes(2);
 		});
 	});

packages/datastore/__tests__/subscription-variables.test.ts

@@ -65,11 +65,8 @@ describe('DataStore Subscription Variables', () => {
 				customVariables,
 			);
 
-			// Verify operation type and name
 			expect(opType).toBe(TransformerMutationType.CREATE);
 			expect(opName).toBe('onCreateTodo');
-
-			// Verify that custom variables are included in the query
 			expect(query).toContain('$storeId: String');
 			expect(query).toContain('$tenantId: String');
 			expect(query).toContain('storeId: $storeId');
@@ -115,11 +112,8 @@ describe('DataStore Subscription Variables', () => {
 				false,
 			);
 
-			// Verify operation type and name
 			expect(opType).toBe(TransformerMutationType.CREATE);
 			expect(opName).toBe('onCreateTodo');
-
-			// Verify that no custom variables are included
 			expect(query).not.toContain('$storeId');
 			expect(query).not.toContain('$tenantId');
 		});

packages/datastore/src/sync/utils.ts

@@ -339,13 +339,10 @@ export function buildSubscriptionGraphQLOperation(
 		opArgs.push(`${ownerField}: $${ownerField}`);
 	}
 
-	// Add custom subscription variables
 	if (customVariables) {
-		// GraphQL variable name must start with letter or underscore, followed by letters, numbers, or underscores
 		const VALID_VAR_NAME = /^[_a-zA-Z][_a-zA-Z0-9]*$/;
 
 		Object.keys(customVariables).forEach(varName => {
-			// Validate variable name
 			if (!VALID_VAR_NAME.test(varName)) {
 				logger.warn(
 					`Invalid GraphQL variable name '${varName}' in subscriptionVariables. Skipping.`,
@@ -354,15 +351,13 @@ export function buildSubscriptionGraphQLOperation(
 				return;
 			}
 
-			// Skip null/undefined values
 			if (
 				customVariables[varName] === null ||
 				customVariables[varName] === undefined
 			) {
 				return;
 			}
 
-			// Infer type from value (simplified - could be enhanced)
 			const varType = Array.isArray(customVariables[varName])
 				? '[String]'
 				: 'String';
@@ -995,7 +990,6 @@ export function getIdentifierValue(
 	return idOrPk;
 }
 
-// Reserved GraphQL variable names that should not be used
 const RESERVED_SUBSCRIPTION_VARIABLE_NAMES = new Set([
 	'input',
 	'condition',
@@ -1037,7 +1031,6 @@ export function processSubscriptionVariables(
 		return undefined;
 	}
 
-	// Check cache first
 	let modelCache = cache.get(model);
 	if (!modelCache) {
 		modelCache = new Map();
@@ -1050,10 +1043,8 @@ export function processSubscriptionVariables(
 		return cached || undefined;
 	}
 
-	// Process the variables
 	let vars: Record<string, any>;
 
-	// Handle function-based variables
 	if (typeof modelVariables === 'function') {
 		try {
 			vars = modelVariables(operation);
@@ -1070,7 +1061,6 @@ export function processSubscriptionVariables(
 		vars = modelVariables;
 	}
 
-	// Validate and sanitize
 	const sanitized = sanitizeSubscriptionVariables(vars, model.name);
 	modelCache.set(operation, sanitized);
 
@@ -1081,7 +1071,6 @@ function sanitizeSubscriptionVariables(
 	vars: any,
 	modelName: string,
 ): Record<string, any> | null {
-	// Validate the input is an object
 	if (vars === null || typeof vars !== 'object' || Array.isArray(vars)) {
 		logger.warn(
 			`subscriptionVariables must be an object for model ${modelName}`,
@@ -1091,12 +1080,10 @@ function sanitizeSubscriptionVariables(
 	}
 
 	try {
-		// Deep clone to prevent mutations
 		const cloned = JSON.parse(JSON.stringify(vars));
 
 		return filterReservedSubscriptionVariableKeys(cloned);
 	} catch {
-		// Can't clone (e.g., circular reference) - use shallow copy
 		return filterReservedSubscriptionVariableKeys({ ...vars });
 	}
 }
@@ -1106,7 +1093,6 @@ function filterReservedSubscriptionVariableKeys(
 ): Record<string, any> | null {
 	const result: Record<string, any> = {};
 
-	// Safe iteration that handles Object.create(null)
 	try {
 		Object.entries(vars).forEach(([key, value]) => {
 			if (!RESERVED_SUBSCRIPTION_VARIABLE_NAMES.has(key)) {
@@ -1118,7 +1104,6 @@ function filterReservedSubscriptionVariableKeys(
 			}
 		});
 	} catch {
-		// Fallback for objects that don't support Object.entries
 		for (const key in vars) {
 			if (
 				Object.prototype.hasOwnProperty.call(vars, key) &&