Clean up Unused Observable Stores - v12.0.0 (#187)

integration-tests/integration.test.js

@@ -1,12 +1,11 @@
 const { getByText, fireEvent, waitFor } = require('@testing-library/dom');
 const { startApp } = require('./test-app');
 
-describe('Tram-One', () => {
-	beforeEach(() => {
-		// clean up any tram-one properties between tests
-		window['tram-space'] = undefined;
-	});
+/**
+ * The following tests are intentional test that validate the behavior of new features.
+ */
 
+describe('Tram-One', () => {
 	it('should render on a Node', () => {
 		// mount the app on the container
 		const container = document.createElement('div');

integration-tests/internals.test.js

@@ -0,0 +1,70 @@
+const { queryByText, fireEvent, waitFor, getByLabelText } = require('@testing-library/dom');
+const { default: userEvent } = require('@testing-library/user-event');
+const { startAppAndWait } = require('./test-helpers');
+
+/**
+ * The following suite of tests verify the behavior of the internals of Tram-One, more so than other tests might.
+ * They are often inpercievable to end-users, and verify the expected behavior of the behind-the-scenes design.
+ */
+
+describe('Tram-One', () => {
+	it('should clean up stores for elements that are no longer rendered', async () => {
+		// start the app
+		const { container } = await startAppAndWait();
+
+		// previously stores made for elements that had been removed stayed in the tram-observable-store
+
+		const initialStores = Object.keys(window['tram-space']['tram-observable-store']);
+
+		// focus on the input (the range input defaults to 0)
+		userEvent.click(getByLabelText(container, 'Store Generator'));
+
+		// change the value of the input
+		fireEvent.change(getByLabelText(container, 'Store Generator'), { target: { value: 1 } });
+
+		await waitFor(() => {
+			// make sure the new control is in the document
+			// (additionally, we're doing this to make sure that all the mutation observers have had a chance to catch up)
+			expect(queryByText(container, '[0: 0]')).toBeVisible();
+		});
+
+		// expect us to have one additional store now
+		const postChangeStores = Object.keys(window['tram-space']['tram-observable-store']);
+		expect(postChangeStores.length).toBe(initialStores.length + 1);
+
+		// change the value of the input back to 0
+		fireEvent.change(getByLabelText(container, 'Store Generator'), { target: { value: 0 } });
+
+		await waitFor(() => {
+			// make sure the new control is in the document
+			// (additionally, we're doing this to make sure that all the mutation observers have had a chance to catch up)
+			expect(queryByText(container, '[0: 0]')).toBe(null);
+		});
+
+		// wait for mutation observer clean up removed stores
+		await waitFor(() => {
+			const postChangeStoresTwo = Object.keys(window['tram-space']['tram-observable-store']);
+			// check that the lists are the same (they may have shuffled, so sort them)
+			expect(postChangeStoresTwo.sort()).toEqual(initialStores.sort());
+		});
+	});
+
+	it('should not have recursive working-key branches', async () => {
+		// start the app
+		await startAppAndWait();
+
+		// previously the working branch indices would have long recursive chains of branches
+
+		const workingKeyBranches = Object.keys(window['tram-space']['tram-hook-key'].branchIndices);
+
+		// verify that top-level elements exist
+		expect(workingKeyBranches).toEqual(expect.arrayContaining(['app[{}]']));
+		expect(workingKeyBranches).toEqual(expect.arrayContaining(['app[{}]/logo[{}]']));
+		expect(workingKeyBranches).toEqual(expect.arrayContaining(['app[{}]/tab[{}]']));
+
+		// verify that no element contains a duplicate of 'app[{}]' - this indicates an issue with the key generation
+		workingKeyBranches.forEach((branch) => {
+			expect(branch).not.toMatch(/app\[\{\}\].*app\[\{\}\]/);
+		});
+	});
+});

integration-tests/regression.test.js

@@ -1,11 +1,16 @@
-const { getByText, queryAllByText, fireEvent, waitFor, getByLabelText } = require('@testing-library/dom');
+const { getByText, queryAllByText, fireEvent, waitFor, getByLabelText, queryByText } = require('@testing-library/dom');
 const { default: userEvent } = require('@testing-library/user-event');
-const { startApp } = require('./test-app');
+const { startAppAndWait } = require('./test-helpers');
+
+/**
+ * The following suite of tests are made retroactively for unexpected behaviors.
+ * They are not for any direct feature, but rather validate the behavior of framework as a whole.
+ */
 
 describe('Tram-One', () => {
 	it('should not call cleanups that are not functions', async () => {
 		// start the app
-		const { container } = startApp();
+		const { container } = await startAppAndWait();
 
 		// previously this would fail because the cleanup was called,
 		// even though it was not a function, and instead was a promise (the result of an async function)
@@ -18,7 +23,7 @@ describe('Tram-One', () => {
 
 	it('should call updated cleanups', async () => {
 		// start the app
-		const { container } = startApp();
+		const { container } = await startAppAndWait();
 
 		// verify that the tab is rendered and the lock button is there
 		expect(getByText(container, 'Was Locked: false')).toBeVisible();
@@ -48,7 +53,7 @@ describe('Tram-One', () => {
 
 	it('should process state as an array', async () => {
 		// start the app
-		const { container } = startApp();
+		const { container } = await startAppAndWait();
 
 		// previously when state was being processed, it would be converted to an object
 		// this test adds an element to a store to verify array methods work
@@ -67,15 +72,15 @@ describe('Tram-One', () => {
 		window.history.pushState({}, '', '/test_account');
 
 		// start the app
-		const { container } = startApp();
+		const { container } = await startAppAndWait();
 
 		// verify the account info is read correctly at startup
 		expect(getByText(container, 'Account: test_account')).toBeVisible();
 	});
 
 	it('should keep focus on inputs when components would rerender', async () => {
 		// start the app
-		const { container } = startApp();
+		const { container } = await startAppAndWait();
 
 		// previously when interacting with an input, if the component would rerender
 		// focus would be removed from the component and put on the body of the page
@@ -89,7 +94,7 @@ describe('Tram-One', () => {
 		});
 
 		// clear the input
-		userEvent.type(getByLabelText(container, 'New Task Label'), '{selectall}{backspace}');
+		userEvent.clear(getByLabelText(container, 'New Task Label'));
 
 		// wait for mutation observer to reapply focus
 		await waitFor(() => {
@@ -111,7 +116,7 @@ describe('Tram-One', () => {
 
 	it('should keep focus on the most recent input when components rerender', async () => {
 		// start the app
-		const { container } = startApp();
+		const { container } = await startAppAndWait();
 
 		// previously when interacting with an input, if the component would rerender
 		// focus would be removed from the component and put on the body of the page
@@ -150,7 +155,7 @@ describe('Tram-One', () => {
 
 	it('should keep focus when both the parent and child element would update', async () => {
 		// start the app
-		const { container } = startApp();
+		const { container } = await startAppAndWait();
 
 		// previously when interacting with an input, if both a parent and child element
 		// would update, then focus would not reattach, and/or the value would not update correctly
@@ -200,7 +205,7 @@ describe('Tram-One', () => {
 
 	it('should not error when resetting focus if the number of elements changed', async () => {
 		// start the app
-		const { container } = startApp();
+		const { container } = await startAppAndWait();
 
 		// previously when interacting with an input, if the number of elements decreased
 		// an error was thrown because the element to focus on no longer existed
@@ -232,11 +237,140 @@ describe('Tram-One', () => {
 
 	it('should trigger use-effects of the first resolved element', async () => {
 		// start the app
-		startApp();
+		await startAppAndWait();
 
 		// previously, useEffects on the first resolved element would not trigger
 		// because the effect queue and effect store were pointed to the same object instance
 
 		expect(document.title).toEqual('Tram-One Testing App');
 	});
+
+	it('should keep focus on inputs without a start and end selection', async () => {
+		// start the app
+		const { container } = await startAppAndWait();
+
+		// previously when interacting with an input of a different type (e.g. range)
+		// when reapplying focus Tram-One would throw an error because while the
+		// function for setting selection range exists, it does not work
+
+		// focus on the input (the range input defaults to 0)
+		userEvent.click(getByLabelText(container, 'Store Generator'));
+
+		// verify that the element has focus (before changing the value)
+		await waitFor(() => {
+			expect(getByLabelText(container, 'Store Generator')).toHaveFocus();
+		});
+
+		// change the value of the input
+		fireEvent.change(getByLabelText(container, 'Store Generator'), { target: { value: 1 } });
+
+		// verify the element has the new value
+		expect(getByLabelText(container, 'Store Generator')).toHaveValue('1');
+
+		// wait for mutation observer to re-attach focus
+		// expect the input to keep focus after the change event
+		await waitFor(() => {
+			expect(getByLabelText(container, 'Store Generator')).toHaveFocus();
+		});
+	});
+
+	it('should not reset stores for elements that are still rendered', async () => {
+		// start the app
+		const { container } = await startAppAndWait();
+
+		// previously state would be blown away if a parent element changed state multiple times
+
+		// focus on the input (the range input defaults to 0)
+		userEvent.click(getByLabelText(container, 'Store Generator'));
+
+		// change the value of the input
+		fireEvent.change(getByLabelText(container, 'Store Generator'), { target: { value: 1 } });
+
+		// click on one of the new stores several times
+		userEvent.click(getByText(container, '[0: 0]'));
+		userEvent.click(getByText(container, '[0: 1]'));
+		userEvent.click(getByText(container, '[0: 2]'));
+		userEvent.click(getByText(container, '[0: 3]'));
+		// the button should now say "[0: 4]"
+		expect(getByText(container, '[0: 4]')).toBeVisible();
+
+		// update the number of stores (the parent store element)
+		fireEvent.change(getByLabelText(container, 'Store Generator'), { target: { value: 2 } });
+
+		// wait for mutation observer clean up removed stores
+		await waitFor(() => {
+			// we should see the new buttons
+			expect(getByText(container, '[1: 0]')).toBeVisible();
+		});
+		fireEvent.change(getByLabelText(container, 'Store Generator'), { target: { value: 3 } });
+		// wait for mutation observer clean up removed stores
+		await waitFor(() => {
+			// we should see the new buttons
+			expect(getByText(container, '[2: 0]')).toBeVisible();
+		});
+
+		// we should still see the button with "4,"
+		expect(getByText(container, '[0: 4]')).toBeVisible();
+	});
+
+	it('should reset stores for elements that have been removed', async () => {
+		// start the app
+		const { container } = await startAppAndWait();
+
+		// previously we would hold on to the local state of elements even if they had been removed
+
+		// focus on the input (the range input defaults to 0)
+		userEvent.click(getByLabelText(container, 'Store Generator'));
+
+		// change the value of the input
+		fireEvent.change(getByLabelText(container, 'Store Generator'), { target: { value: 5 } });
+
+		// expect to see all the stores with their initial values
+		await waitFor(() => {
+			expect(getByText(container, '[0: 0]')).toBeVisible();
+			expect(getByText(container, '[1: 0]')).toBeVisible();
+			expect(getByText(container, '[2: 0]')).toBeVisible();
+			expect(getByText(container, '[3: 0]')).toBeVisible();
+			expect(getByText(container, '[4: 0]')).toBeVisible();
+		});
+
+		// click on each of the new stores
+		userEvent.click(getByText(container, '[0: 0]'));
+		userEvent.click(getByText(container, '[1: 0]'));
+		userEvent.click(getByText(container, '[2: 0]'));
+		userEvent.click(getByText(container, '[3: 0]'));
+		userEvent.click(getByText(container, '[4: 0]'));
+
+		// expect to see all the stores with the new values
+		await waitFor(() => {
+			expect(getByText(container, '[0: 1]')).toBeVisible();
+			expect(getByText(container, '[1: 1]')).toBeVisible();
+			expect(getByText(container, '[2: 1]')).toBeVisible();
+			expect(getByText(container, '[3: 1]')).toBeVisible();
+			expect(getByText(container, '[4: 1]')).toBeVisible();
+		});
+
+		// remove all of the stores by setting the value to 0
+		fireEvent.change(getByLabelText(container, 'Store Generator'), { target: { value: 0 } });
+
+		await waitFor(() => {
+			expect(queryByText(container, '[0: 1]')).toBe(null);
+			expect(queryByText(container, '[1: 1]')).toBe(null);
+			expect(queryByText(container, '[2: 1]')).toBe(null);
+			expect(queryByText(container, '[3: 1]')).toBe(null);
+			expect(queryByText(container, '[4: 1]')).toBe(null);
+		});
+
+		// re-add the stores by setting the value to 5
+		fireEvent.change(getByLabelText(container, 'Store Generator'), { target: { value: 5 } });
+
+		// expect to see all the stores with their initial values
+		await waitFor(() => {
+			expect(getByText(container, '[0: 0]')).toBeVisible();
+			expect(getByText(container, '[1: 0]')).toBeVisible();
+			expect(getByText(container, '[2: 0]')).toBeVisible();
+			expect(getByText(container, '[3: 0]')).toBeVisible();
+			expect(getByText(container, '[4: 0]')).toBeVisible();
+		});
+	});
 });

integration-tests/test-app/element-store-generator.ts

@@ -0,0 +1,34 @@
+import { registerHtml, useStore, TramOneComponent } from '../../src/tram-one';
+import elementwithstore from './element-with-store';
+
+const html = registerHtml({
+	elementwithstore,
+});
+
+/**
+ * Element to verify non-standard input controls, and also verify memory leak type issues
+ */
+const elementstoregenerator: TramOneComponent = () => {
+	const storeGeneratorStore = useStore({ count: 0 });
+	const incrementCount = (event: InputEvent) => {
+		const inputElement = event.target as HTMLInputElement;
+		storeGeneratorStore.count = parseInt(inputElement.value);
+	};
+	const storeElements = [...new Array(storeGeneratorStore.count)].map((_, index) => {
+		return html`<elementwithstore index=${index} />`;
+	});
+	return html`<section>
+		<label for="store-generator">Store Generator</label>
+		<input
+			id="store-generator"
+			type="range"
+			min="0"
+			max="5"
+			value=${storeGeneratorStore.count}
+			onchange=${incrementCount}
+		/>
+		${storeElements}
+	</section>`;
+};
+
+export default elementstoregenerator;

integration-tests/test-app/element-with-store.ts

@@ -0,0 +1,14 @@
+import { registerHtml, useStore, TramOneComponent } from '../../src/tram-one';
+
+const html = registerHtml();
+
+/**
+ * Dynamicly generated component that could possibly cause memory leaks
+ */
+const elementwithstore: TramOneComponent = ({ index }) => {
+	const subElementStore = useStore({ count: 0 });
+	const onIncrement = () => subElementStore.count++;
+	return html` <button onclick=${onIncrement}>[${index}: ${subElementStore.count}]</button> `;
+};
+
+export default elementwithstore;

integration-tests/test-app/index.html

@@ -3,9 +3,9 @@
 	<head>
 		<meta charset="utf-8" />
 		<style>
-			body { background: #0a0f21; color: #cbcbcb; }
-			input { background-color: inherit; color: inherit; }
-			button { background: inherit; color: inherit; }
+			:root {
+				color-scheme: dark light;
+			}
 		</style>
 	</head>
 	<body>