Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Existing Effects & Refs #196

Merged
merged 10 commits into from
Jun 30, 2022
11 changes: 10 additions & 1 deletion integration-tests/integration.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const { getByText, fireEvent, waitFor } = require('@testing-library/dom');
const { getByText, fireEvent, waitFor, getByPlaceholderText } = require('@testing-library/dom');
const { startApp } = require('./test-app');
const { startAppAndWait } = require('./test-helpers');

/**
* The following tests are intentional test that validate the behavior of new features.
Expand Down Expand Up @@ -156,4 +157,12 @@ describe('Tram-One', () => {
// verify the account info updated
expect(getByText(container, 'Is Account Logged In: Yes')).toBeVisible();
});

it('should process effects on the element passed through use-effect', async () => {
// start the app
const { container } = await startAppAndWait();

// verify the effect properly focused the input element
expect(getByPlaceholderText(container, 'Input for automatic focus')).toHaveFocus();
});
});
11 changes: 11 additions & 0 deletions integration-tests/regression.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -343,4 +343,15 @@ describe('Tram-One', () => {
expect(getByText(container, '[4: 0]')).toBeVisible();
});
});

it('should process effects of components that return other components at root', async () => {
// start the app
await startAppAndWait();

// previously if an element immediately returned another component,
// the effects of the child component would be lost

// verify that effects were trigged
expect(window.location.hash).toBe('#testing');
});
});
15 changes: 15 additions & 0 deletions integration-tests/test-app/anchor-set.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { registerHtml, useEffect, TramOneComponent } from '../../src/tram-one';

const html = registerHtml();

/**
* Element with a use effect (to test behavior when wrapped)
*/
const anchorSet: TramOneComponent = () => {
useEffect(() => {
window.location.hash = 'testing';
});
return html`<section />`;
};

export default anchorSet;
16 changes: 16 additions & 0 deletions integration-tests/test-app/anchors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { registerHtml, useUrlParams, TramOneComponent } from '../../src/tram-one';
import anchorSet from './anchor-set';

const html = registerHtml({
'anchor-set': anchorSet,
});

/**
* Element to test behavior or wrapping a single component
*/
const anchors: TramOneComponent = () => {
if (useUrlParams().hash !== 'testing') return html`<anchor-set />`;
return html`<section>Anchor Set</section>`;
};

export default anchors;
15 changes: 15 additions & 0 deletions integration-tests/test-app/focus-input.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { registerHtml, useEffect, TramOneComponent } from '../../src/tram-one';

const html = registerHtml();

/**
* Element to test useEffect reference point
*/
const focusInput: TramOneComponent = () => {
useEffect((ref) => {
ref.focus();
});
return html`<input placeholder="Input for automatic focus" />`;
};

export default focusInput;
6 changes: 6 additions & 0 deletions integration-tests/test-app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import mirrorinput from './mirror-input';
import documentTitleSetter from './document-title-setter';
import elementstoregenerator from './element-store-generator';
import { TramWindow } from '../../src/types';
import anchors from './anchors';
import focusInput from './focus-input';

const html = registerHtml({
title: title,
Expand All @@ -22,6 +24,8 @@ const html = registerHtml({
'mirror-input': mirrorinput,
'document-title-setter': documentTitleSetter,
'element-store-generator': elementstoregenerator,
anchors: anchors,
'focus-input': focusInput,
});

/**
Expand All @@ -41,6 +45,7 @@ export const app = () => {
<document-title-setter />

<title subtitle="Sub Title Prop">Sub Title Child</title>
<focus-input />
<p>Root Loaded: ${rootStore.loaded}</p>
<logo />
<account />
Expand All @@ -51,6 +56,7 @@ export const app = () => {
<tasks />
<mirror-input />
<element-store-generator />
<anchors />
</main>
`;
};
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "tram-one",
"version": "13.0.0",
"version": "13.1.0",
"description": "🚋 Modern View Framework for Vanilla Javascript",
"main": "dist/tram-one.cjs",
"commonjs": "dist/tram-one.cjs",
Expand Down
6 changes: 3 additions & 3 deletions src/dom-wrappers.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { registerDom } from './dom';

import { Registry } from './types';
import { Registry, TramOneHTMLElement, TramOneSVGElement } from './types';

/**
* @name registerHtml
Expand All @@ -13,7 +13,7 @@ import { Registry } from './types';
* @return tagged template function that builds HTML components
*/
export const registerHtml = (registry?: Registry) => {
return registerDom(null, registry);
return registerDom<TramOneHTMLElement>(null, registry);
};

/**
Expand All @@ -26,5 +26,5 @@ export const registerHtml = (registry?: Registry) => {
* @return tagged template function that builds SVG components
*/
export const registerSvg = (registry?: Registry) => {
return registerDom('http://www.w3.org/2000/svg', registry);
return registerDom<TramOneSVGElement>('http://www.w3.org/2000/svg', registry);
};
12 changes: 8 additions & 4 deletions src/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import observeTag from './observe-tag';
import processHooks from './process-hooks';
import { TRAM_TAG, TRAM_TAG_NEW_EFFECTS, TRAM_TAG_CLEANUP_EFFECTS } from './node-names';

import { Registry, Props, DOMTaggedTemplateFunction, Children } from './types';
import { Registry, Props, DOMTaggedTemplateFunction, Children, TramOneHTMLElement, TramOneSVGElement } from './types';

/**
* This function takes in a namespace and registry of custom components,
Expand All @@ -25,7 +25,10 @@ import { Registry, Props, DOMTaggedTemplateFunction, Children } from './types';
* @param registry mapping of tag names to component functions
* @param namespace namespace to create nodes in (by default XHTML namespace)
*/
export const registerDom = (namespace: string | null, registry: Registry = {}): DOMTaggedTemplateFunction => {
export const registerDom = <ElementType extends TramOneHTMLElement | TramOneSVGElement>(
namespace: string | null,
registry: Registry = {}
) => {
// modify the registry so that each component function updates the hook working key
const hookedRegistry = Object.keys(registry).reduce((newRegistry, tagName) => {
const tagFunction = registry[tagName];
Expand Down Expand Up @@ -59,13 +62,14 @@ export const registerDom = (namespace: string | null, registry: Registry = {}):
tagResult[TRAM_TAG] = true;
// we won't decorate TRAM_TAG_REACTION, that needs to be done later when we observe the tag
tagResult[TRAM_TAG_NEW_EFFECTS] = tagResult[TRAM_TAG_NEW_EFFECTS] || [];
tagResult[TRAM_TAG_CLEANUP_EFFECTS] = tagResult[TRAM_TAG_NEW_EFFECTS] || [];
// cleanup effects will be populated when new effects are processed
tagResult[TRAM_TAG_CLEANUP_EFFECTS] = [];
Comment on lines +65 to +66
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's no reason this should have been populated by the existing new effects before...


return tagResult;
};

return { ...newRegistry, [tagName]: hookedTagFunction };
}, {});

return rbel(hyperx, nanohtml(namespace), hookedRegistry);
return rbel(hyperx, nanohtml(namespace), hookedRegistry) as DOMTaggedTemplateFunction<ElementType>;
};
2 changes: 1 addition & 1 deletion src/mount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ export default (component: RootTramOneComponent, container: Container) => {

// this sadly needs to be wrapped in some element so we can process effects
// otherwise the root node will not have effects applied on it
const renderedApp = html`<div><app /></div>`;
const renderedApp = html`<tram-one><app /></tram-one>`;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the UI, this appears like the following:
image

This is better than <div>, because we won't inherit any unexpected styles or formatting.

container.replaceChild(renderedApp, container.firstElementChild);
};
8 changes: 4 additions & 4 deletions src/mutation-observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
TRAM_TAG_STORE_KEYS,
} from './node-names';
import { buildNamespace } from './namespace';
import { TramOneElement } from './types';
import { CleanupEffect, TramOneElement, TramOneHTMLElement, TramOneSVGElement } from './types';
import { getObservableStore } from './observable-store';
import { TRAM_OBSERVABLE_STORE, TRAM_KEY_STORE } from './engine-names';
import { decrementKeyStoreValue, getKeyStore, incrementKeyStoreValue } from './key-store';
Expand All @@ -25,7 +25,7 @@ import { decrementKeyStoreValue, getKeyStore, incrementKeyStoreValue } from './k
* process side-effects for new tram-one nodes
* (this includes calling effects, and keeping track of stores)
*/
const processTramTags = (node: Node | TramOneElement) => {
const processTramTags = (node: Node | (TramOneHTMLElement | TramOneSVGElement)) => {
// if this element doesn't have a TRAM_TAG, it's not a Tram-One Element
if (!(TRAM_TAG in node)) {
return;
Expand Down Expand Up @@ -55,7 +55,7 @@ const processTramTags = (node: Node | TramOneElement) => {
const effectReaction = observe(() => {
// verify that cleanup is a function before calling it (in case it was a promise)
if (typeof cleanup === 'function') cleanup();
cleanup = effect();
cleanup = effect(node);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is what is required for allowing us to accept ref as a parameter for useEffect - we pass the node we are processing back into the effect it defined!

});

// this is called when a component with an effect is removed
Expand All @@ -76,7 +76,7 @@ const processTramTags = (node: Node | TramOneElement) => {
/**
* call all cleanup effects on the node
*/
const cleanupEffects = (cleanupEffects: (() => void)[]) => {
const cleanupEffects = (cleanupEffects: CleanupEffect[]) => {
cleanupEffects.forEach((cleanup) => cleanup());
};

Expand Down
20 changes: 15 additions & 5 deletions src/process-hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,25 @@ export default (tagFunction: () => TramOneElement) => {
const existingEffects = getEffectStore(TRAM_EFFECT_STORE);
const queuedEffects = getEffectStore(TRAM_EFFECT_QUEUE);

// get all new keys
const newKeys = getKeyQueue(TRAM_KEY_QUEUE);
// pull new effects that have yet to be processed from the tag
// these can appear when a component re-exposes another component at its root
const existingNewEffects = tagResult[TRAM_TAG_NEW_EFFECTS] || [];

// store new effects in the node we just built
// store new effects (and the existing new effects) in the node we just built
const newEffects = Object.keys(queuedEffects).filter((effect) => !(effect in existingEffects));
tagResult[TRAM_TAG_NEW_EFFECTS] = newEffects.map((newEffectKey) => queuedEffects[newEffectKey]);
const newEffectFunctions = newEffects.map((newEffectKey) => queuedEffects[newEffectKey]);
const existingNewAndBrandNewEffects = existingNewEffects.concat(newEffectFunctions);
JRJurman marked this conversation as resolved.
Show resolved Hide resolved
tagResult[TRAM_TAG_NEW_EFFECTS] = existingNewAndBrandNewEffects;

// same as the existingNewEffects, but for state values
const existingNewKeys = tagResult[TRAM_TAG_STORE_KEYS] || [];

// get all new keys
const newKeys = getKeyQueue(TRAM_KEY_QUEUE);

// store keys in the node we just built
tagResult[TRAM_TAG_STORE_KEYS] = newKeys;
const existingNewAndBrandNewKeys = existingNewKeys.concat(newKeys);
tagResult[TRAM_TAG_STORE_KEYS] = existingNewAndBrandNewKeys;

// restore the effect and key queues to what they were before we started
restoreEffectStore(TRAM_EFFECT_QUEUE, existingQueuedEffects);
Expand Down
9 changes: 6 additions & 3 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ export type ElementOrSelector = [string | HTMLElement][0];
* Type for our template renderers (either html or svg).
* This is not wrapped in an indexed alias, because everything should be provided automatically.
*/
export type DOMTaggedTemplateFunction = (
export type DOMTaggedTemplateFunction<TramOneElementType extends TramOneElement> = (
strings: TemplateStringsArray,
...elementsAndAttributes: any[]
) => TramOneElement;
) => TramOneElementType;

/**
* Type for custom Tram One Components.
Expand Down Expand Up @@ -54,7 +54,7 @@ export type CleanupEffect = [() => unknown][0];
* Type for the effect function.
* This is passed into the useEffect hook
*/
export type Effect = [() => unknown][0];
export type Effect = [(ref: TramOneHTMLElement | TramOneSVGElement) => unknown][0];

/**
* The Props interface for custom Tram One Components.
Expand Down Expand Up @@ -108,6 +108,9 @@ export interface TramOneElement extends Element {
[TRAM_TAG_STORE_KEYS]: string[];
}

export type TramOneHTMLElement = TramOneElement & HTMLElement;
export type TramOneSVGElement = TramOneElement & SVGElement;

Comment on lines +111 to +113
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new type definitions, because TramOneElement is too generic, and in reality (right now) it's always an HTMLElement or SVGElement.

If we ever expose registerDOM, people should be able to make their own TramOneElement definitions (which shouldn't be too hard to do).

/**
* Type for the Root TramOneComponent,
* it can have no props or children, since it is the root element
Expand Down
2 changes: 1 addition & 1 deletion src/use-url-params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { UrlMatchResults } from './types';
*
* @returns object with a `matches` key, and (if it matched) path and query parameters
*/
export default (pattern: string): UrlMatchResults => {
export default (pattern?: string): UrlMatchResults => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should have always been optional! This better matches the interface of the underlying library.

// save and update results in an observable, so that we can update
// components and effects in a reactive way
const initialParams = useUrlParams(pattern) as UrlMatchResults;
Expand Down