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

[DRAFT] Add twoPass option for additional state retention #72

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
14 changes: 14 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,17 @@ jobs:
- name: Run tests
run: npm run ci

test-move-before:
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v4
- name: Use Node.js
uses: actions/setup-node@v4
with:
cache: 'npm'
- name: Install dependencies
run: npm install
- name: Run tests
run: npm run test-move-before

1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/node_modules/
/coverage
/test/chrome-profile
.idea
6 changes: 6 additions & 0 deletions TESTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ npm run ci
```
This will run the tests using Playwright’s headless browser setup across Chrome, Firefox, and WebKit (Safari-adjacent). This is ultimately what gets run in Github Actions to verify PRs.

To run all tests against Chrome with experimental `moveBefore` support added, execute:
```bash
npm run test-move-before
```
This will start headless Chrome in a new profile with the `atomic-move` experimental flag set. This runs in a separate job in CI.

## Running Individual Tests

### Headless Mode
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"scripts": {
"test": "web-test-runner",
"debug": "web-test-runner --manual --open",
"test-move-before": "USE_MOVE_BEFORE=1 web-test-runner",
"ci": "web-test-runner --playwright --browsers chromium firefox webkit",
"amd": "(echo \"define(() => {\n\" && cat src/idiomorph.js && echo \"\nreturn Idiomorph});\") > dist/idiomorph.amd.js",
"cjs": "(cat src/idiomorph.js && echo \"\nmodule.exports = Idiomorph;\") > dist/idiomorph.cjs.js",
Expand Down
103 changes: 97 additions & 6 deletions src/idiomorph.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
* @property {boolean} [ignoreActiveValue]
* @property {ConfigCallbacksInternal} callbacks
* @property {ConfigHeadInternal} head
* @property {boolean} [twoPass]
*/

/**
Expand All @@ -78,7 +79,7 @@
* @param {Element | Document} oldNode
* @param {Element | Node | HTMLCollection | Node[] | string | null} newContent
* @param {Config} [config]
* @returns {undefined | HTMLCollection | Node[]}
* @returns {undefined | Node[]}
*/

// base IIFE to define idiomorph
Expand All @@ -102,6 +103,7 @@ var Idiomorph = (function () {
* @property {Set<string>} deadIds
* @property {ConfigInternal['callbacks']} callbacks
* @property {ConfigInternal['head']} head
* @property {boolean|HTMLDivElement} [pantry]
*/

//=============================================================================
Expand Down Expand Up @@ -152,7 +154,7 @@ var Idiomorph = (function () {
* @param {Element | Document} oldNode
* @param {Element | Node | HTMLCollection | Node[] | string | null} newContent
* @param {Config} [config]
* @returns {undefined | HTMLCollection | Node[]}
* @returns {undefined | Node[]}
*/
function morph(oldNode, newContent, config = {}) {

Expand All @@ -176,7 +178,7 @@ var Idiomorph = (function () {
* @param {Element} oldNode
* @param {Element} normalizedNewContent
* @param {MorphContext} ctx
* @returns {undefined | HTMLCollection| Node[]}
* @returns {undefined | Node[]}
*/
function morphNormalizedContent(oldNode, normalizedNewContent, ctx) {
if (ctx.head.block) {
Expand All @@ -201,6 +203,9 @@ var Idiomorph = (function () {

// innerHTML, so we are only updating the children
morphChildren(normalizedNewContent, oldNode, ctx);
if (ctx.pantry) {
restoreFromPantry(oldNode, ctx);
}
return Array.from(oldNode.children);

} else if (ctx.morphStyle === "outerHTML" || ctx.morphStyle == null) {
Expand All @@ -219,7 +224,11 @@ var Idiomorph = (function () {
// if there was a best match, merge the siblings in too and return the
// whole bunch
if (morphedNode) {
return insertSiblings(previousSibling, morphedNode, nextSibling);
const elements = insertSiblings(previousSibling, morphedNode, nextSibling);
if (ctx.pantry) {
restoreFromPantry(morphedNode.parentNode, ctx);
}
return elements
}
} else {
// otherwise nothing was added to the DOM
Expand Down Expand Up @@ -713,11 +722,19 @@ var Idiomorph = (function () {
ignoreActiveValue: mergedConfig.ignoreActiveValue,
idMap: createIdMap(oldNode, newContent),
deadIds: new Set(),
pantry: mergedConfig.twoPass && createPantry(),
callbacks: mergedConfig.callbacks,
head: mergedConfig.head
}
}

function createPantry() {
const pantry = document.createElement("div");
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe add style='display:none' to hide?

Copy link
Contributor

Choose a reason for hiding this comment

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

also maybe throw an id on it? idiomorph-pantry?

Copy link
Author

@botandrose botandrose Dec 13, 2024

Choose a reason for hiding this comment

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

maybe add style='display:none' to hide?

IDK, el.hidden = true on the next line seems to be a higher level way to express the same thing as style="display: none", and is just as good in terms of browser support https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/hidden#browser_compatibility

I'm not married to it, though. If you prefer the latter that's fine.

Copy link
Author

@botandrose botandrose Dec 13, 2024

Choose a reason for hiding this comment

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

also maybe throw an id on it? idiomorph-pantry?

I actually had this at first, but ultimately decided to remove it, thinking that it could get weird with id collision if we have more than one instance of Idiomorph running on a page simultaneously, which seems plausible when used in the context of Turbo or htmx. I'm not imagining it buying us much, either, aside from perhaps someone being able to pick it out more easily in the dev tools html inspector while debugging. What do you think?

pantry.hidden = true;
document.body.insertAdjacentElement("afterend", pantry);
return pantry;
}

/**
*
* @param {Node | null} node1
Expand Down Expand Up @@ -752,6 +769,9 @@ var Idiomorph = (function () {
if (node1 == null || node2 == null) {
return false;
}
if (node1.id !== node2.id) {
return false;
}
return node1.nodeType === node2.nodeType &&
// ok to cast: if one is not element, `tagName` will be undefined and we'll compare that
/** @type {Element} */ (node1).tagName === /** @type {Element} */ (node2).tagName
Expand Down Expand Up @@ -1063,11 +1083,82 @@ var Idiomorph = (function () {
function removeNode(tempNode, ctx) {
removeIdsFromConsideration(ctx, tempNode)
if (ctx.callbacks.beforeNodeRemoved(tempNode) === false) return;

tempNode.parentNode?.removeChild(tempNode);
if (ctx.pantry && tempNode instanceof Element) {
moveToPantry(tempNode, ctx);
} else {
tempNode.parentNode?.removeChild(tempNode);
}
ctx.callbacks.afterNodeRemoved(tempNode);
}

/**
*
* @param {Element} node
* @param {MorphContext} ctx
*/
function moveToPantry(node, ctx) {
if (ctx.pantry instanceof HTMLDivElement) {
// If the node is a leaf (no children), process it, and then we're done
if (!node.hasChildNodes()) {
if (node.id) {
// @ts-ignore - use proposed moveBefore feature
if (ctx.pantry.moveBefore) {
// @ts-ignore - use proposed moveBefore feature
ctx.pantry.moveBefore(node, null);
} else {
ctx.pantry.insertBefore(node, null);
}
}

// otherwise we need to process the children first
} else {
Array.from(node.children).forEach(child => {
moveToPantry(child, ctx);
});

// After processing children, process the current node
if (node.id) {
node.innerHTML = '';
ctx.pantry.appendChild(node);
} else {
node.parentNode?.removeChild(node);
}
}
botandrose marked this conversation as resolved.
Show resolved Hide resolved
}
}

/**
*
* @param {Node | null} root
* @param {MorphContext} ctx
*/
function restoreFromPantry(root, ctx) {
if (ctx.pantry instanceof HTMLDivElement && root instanceof Element) {
Array.from(ctx.pantry.children).reverse().forEach(element => {
const matchElement = root.querySelector(`#${element.id}`);
if (matchElement) {
// @ts-ignore - use proposed moveBefore feature
if (matchElement.parentElement?.moveBefore) {
// @ts-ignore - use proposed moveBefore feature
matchElement.parentElement.moveBefore(element, matchElement);
while (matchElement.hasChildNodes()) {
// @ts-ignore - use proposed moveBefore feature
element.moveBefore(matchElement.firstChild,null);
}
} else {
matchElement.before(element);
while (matchElement.firstChild) {
element.insertBefore(matchElement.firstChild,null)
}
}
syncNodeFrom(matchElement, element, ctx);
matchElement.remove();
}
});
ctx.pantry.remove();
}
}

//=============================================================================
// ID Set Functions
//=============================================================================
Expand Down
4 changes: 2 additions & 2 deletions test/bootstrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,15 @@ describe("Bootstrap test", function(){
let d2 = div1.querySelector("#d2")
let d3 = div1.querySelector("#d3")

let morphTo = '<div id="root2"><div><div id="d2">E</div></div><div><div id="d3">F</div></div><div><div id="d1">D</div></div></div>';
let morphTo = '<div id="root1"><div><div id="d2">E</div></div><div><div id="d3">F</div></div><div><div id="d1">D</div></div></div>';
let div2 = make(morphTo)

print(div1);
Idiomorph.morph(div1, div2);
print(div1);

// first paragraph should have been discarded in favor of later matches
d1.innerHTML.should.equal("A");
d1.innerHTML.should.not.equal("D");

// second and third paragraph should have morphed
d2.innerHTML.should.equal("E");
Expand Down
2 changes: 1 addition & 1 deletion test/fidelity.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe("Tests to ensure that idiomorph merges properly", function(){
it('morphs multiple attributes correctly twice', function ()
{
const a = `<section class="child">A</section>`;
const b = `<section class="thing" data-one="1" data-two="2" data-three="3" data-four="4" id="foo" fizz="buzz" foo="bar">B</section>`;
const b = `<section class="thing" data-one="1" data-two="2" data-three="3" data-four="4" fizz="buzz" foo="bar">B</section>`;
const expectedA = make(a);
const expectedB = make(b);
const initial = make(a);
Expand Down
55 changes: 55 additions & 0 deletions test/htmx-integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,59 @@ describe("Tests for the htmx integration", function() {
initialBtn.classList.contains('bar').should.equal(true);
});

it('plays nice with hx-preserve', function() {
this.server.respondWith("GET", "/test", "<div><input id='input' hx-preserve><button id='b1' hx-swap='morph' hx-get='/test'>Foo</button><div>");
let html = makeForHtmxTest("<div><input id='input' hx-preserve value='preserve-me!'><button id='b1' hx-swap='morph' hx-get='/test'>Foo</button><div>");
let button = document.getElementById('b1');
button.click();
this.server.respond();
let input = document.getElementById('input');
input.value.should.equal('preserve-me!');
});

it('plays nice with swapping preserved inputs', function() {
this.server.respondWith("GET", "/test", `
<div>
<input id='second' hx-preserve>
<input id='first' hx-preserve>
<button id='b1' hx-swap='morph' hx-get='/test'>Swap</button>
<div>
`);
let html = makeForHtmxTest(`
<div>
<input id='first' hx-preserve>
<input id='second' hx-preserve>
<button id='b1' hx-swap='morph' hx-get='/test'>Swap</button>
<div>
`);
document.getElementById('first').value = 'preserve first!';
document.getElementById('second').value = 'preserve second!';
let button = document.getElementById('b1');
button.click();
this.server.respond();
Array.from(html.querySelectorAll("input")).map(e => e.value).should.eql(['preserve second!', 'preserve first!']);
});

it('plays nice with swapping preserved textareas', function() {
this.server.respondWith("GET", "/test", `
<div>
<textarea id='second' hx-preserve></textarea>
<textarea id='first' hx-preserve></textarea>
<button id='b1' hx-swap='morph' hx-get='/test'>Swap</button>
<div>
`);
let html = makeForHtmxTest(`
<div>
<textarea id='first' hx-preserve></textarea>
<textarea id='second' hx-preserve></textarea>
<button id='b1' hx-swap='morph' hx-get='/test'>Swap</button>
<div>
`);
document.getElementById('first').innerHTML = 'preserve first!';
document.getElementById('second').innerHTML = 'preserve second!';
let button = document.getElementById('b1');
button.click();
this.server.respond();
Array.from(html.querySelectorAll("textarea")).map(e => e.value).should.eql(['preserve second!', 'preserve first!']);
});
})
Loading
Loading