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.

Sign up for GitHub

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

Jump to bottom

Fix failing tests - account for the morph function now returning a promise #2528

Merged
merged 2 commits into from Mar 1, 2022
Merged

Fix failing tests - account for the morph function now returning a promise #2528

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0265333
wip: fix failing tests - account for the morph function now returning…
VicGUTT Dec 25, 2021
7f52b82
fix: failing tests
VicGUTT Feb 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 6 additions & 4 deletions packages/morph/src/morph.js
View file
Open in desktop
Expand Up @@ -25,7 +25,7 @@ export async function morph(from, toHtml, options) {

await breakpoint()

patch(from, toEl)
await patch(from, toEl)

return from
}
Expand Down Expand Up @@ -57,7 +57,7 @@ function assignOptions(options = {}) {
adding = options.adding || noop
added = options.added || noop
key = options.key || defaultGetKey
lookahead = options.lookahead || true
lookahead = options.lookahead || false
debug = options.debug || false
}

Expand Down Expand Up @@ -190,12 +190,12 @@ async function patchChildren(from, to) {
if (toKey && domKeyHoldovers[toKey]) {
let holdover = domKeyHoldovers[toKey]

dom.append(from, holdover)
dom(from).append(holdover)
currentFrom = holdover

await breakpoint('Add element (from key)')
} else {
let added = addNodeTo(currentTo, from)
let added = addNodeTo(currentTo, from) || {}

await breakpoint('Add element: ' + added.outerHTML || added.nodeValue)

Expand Down Expand Up @@ -319,6 +319,8 @@ function addNodeTo(node, parent) {

return clone
}

return null;
}

function addNodeBefore(node, beforeMe) {
Expand Down
4 changes: 2 additions & 2 deletions tests/jest/morph/external.spec.js
View file
Open in desktop
Expand Up @@ -36,6 +36,6 @@ test('change attribute', () => assertPatch(
`<div foo="baz">foo</div>`
))

function assertPatch(before, after) {
expect(morph(createElement(before), after).outerHTML).toEqual(after)
async function assertPatch(before, after) {
expect((await morph(createElement(before), after)).outerHTML).toEqual(after)
}
56 changes: 28 additions & 28 deletions tests/jest/morph/hooks.spec.js
View file
Open in desktop
@@ -1,22 +1,22 @@
let { morph } = require('@alpinejs/morph')
let createElement = require('./createElement.js')

test('can use custom key name', () => {
test('can use custom key name', async () => {
let dom = createElement('<ul><li wire:key="2">bar</li></ul>')

dom.querySelector('li').is_me = true

morph(dom, '<ul><li wire:key="1">foo</li><li wire:key="2">bar</li></ul>', {
await morph(dom, '<ul><li wire:key="1">foo</li><li wire:key="2">bar</li></ul>', {
key(el) { return el.getAttribute('wire:key') }
})

expect(dom.querySelector('li:nth-of-type(2)').is_me).toBeTruthy()
})

test('can prevent update', () => {
test('can prevent update', async () => {
let dom = createElement('<div><span>foo</span></div>')

morph(dom, '<div><span>bar</span></div>', {
await morph(dom, '<div><span>bar</span></div>', {
updating(from, to, childrenOnly, prevent) {
if (from.textContent === 'foo') {
prevent()
Expand All @@ -27,10 +27,10 @@ test('can prevent update', () => {
expect(dom.querySelector('span').textContent).toEqual('foo')
})

test('can prevent update, but still update children', () => {
test('can prevent update, but still update children', async () => {
let dom = createElement('<div><span foo="bar">foo</span></div>')

morph(dom, '<div><span foo="baz">bar</span></div>', {
await morph(dom, '<div><span foo="baz">bar</span></div>', {
updating(from, to, childrenOnly, prevent) {
if (from.textContent === 'foo') {
childrenOnly()
Expand All @@ -42,12 +42,12 @@ test('can prevent update, but still update children', () => {
expect(dom.querySelector('span').getAttribute('foo')).toEqual('bar')
})

test('changing tag doesnt trigger an update (add and remove instead)', () => {
test('changing tag doesnt trigger an update (add and remove instead)', async () => {
let dom = createElement('<div><span>foo</span></div>')

let updateHookCalledTimes = 0

morph(dom, '<div><h1>foo</h1></div>', {
await morph(dom, '<div><h1>foo</h1></div>', {
updating(from, to, prevent) {
updateHookCalledTimes++
}
Expand All @@ -56,10 +56,10 @@ test('changing tag doesnt trigger an update (add and remove instead)', () => {
expect(updateHookCalledTimes).toEqual(1)
})

test('can impact update', () => {
test('can impact update', async () => {
let dom = createElement('<div><span>foo</span></div>')

morph(dom, '<div><span>bar</span></div>', {
await morph(dom, '<div><span>bar</span></div>', {
updated(from, to) {
if (from.textContent === 'bar') {
from.textContent = 'baz'
Expand All @@ -70,13 +70,13 @@ test('can impact update', () => {
expect(dom.querySelector('span').textContent).toEqual('baz')
})

test('updating and updated are sequential when element has child updates ', () => {
test('updating and updated are sequential when element has child updates ', async () => {
let dom = createElement('<div><span>foo</span></div>')

let updatings = []
let updateds = []

morph(dom, '<div><span>bar</span></div>', {
await morph(dom, '<div><span>bar</span></div>', {
updating(from, to) {
updatings.push(from.nodeName.toLowerCase())
},
Expand All @@ -90,10 +90,10 @@ test('updating and updated are sequential when element has child updates ', () =
expect(updateds).toEqual(['div', 'span', '#text'])
})

test('can prevent removal', () => {
test('can prevent removal', async () => {
let dom = createElement('<div><span>foo</span></div>')

morph(dom, '<div></div>', {
await morph(dom, '<div></div>', {
removing(from, prevent) {
prevent()
}
Expand All @@ -102,12 +102,12 @@ test('can prevent removal', () => {
expect(dom.querySelector('span')).toBeTruthy()
})

test('can impact removal', () => {
test('can impact removal', async () => {
let dom = createElement('<div><span>foo</span></div>')

let textContent

morph(dom, '<div></div>', {
await morph(dom, '<div></div>', {
removed(from) {
textContent = from.textContent
}
Expand All @@ -116,10 +116,10 @@ test('can impact removal', () => {
expect(textContent).toEqual('foo')
})

test('can prevent removal for tag replacement', () => {
test('can prevent removal for tag replacement', async () => {
let dom = createElement('<div><span>foo</span></div>')

morph(dom, '<div><h1>foo</h1></div>', {
await morph(dom, '<div><h1>foo</h1></div>', {
removing(from, prevent) {
prevent()
}
Expand All @@ -128,12 +128,12 @@ test('can prevent removal for tag replacement', () => {
expect(dom.querySelector('span')).toBeTruthy()
})

test('can impact removal for tag replacement', () => {
test('can impact removal for tag replacement', async () => {
let dom = createElement('<div><span>foo</span></div>')

let textContent

morph(dom, '<div><h1>foo</h1></div>', {
await morph(dom, '<div><h1>foo</h1></div>', {
removed(from) {
textContent = from.textContent
}
Expand All @@ -142,10 +142,10 @@ test('can impact removal for tag replacement', () => {
expect(textContent).toEqual('foo')
})

test('can prevent addition', () => {
test('can prevent addition', async () => {
let dom = createElement('<div></div>')

morph(dom, '<div><span>foo</span></div>', {
await morph(dom, '<div><span>foo</span></div>', {
adding(to, prevent) {
prevent()
}
Expand All @@ -154,10 +154,10 @@ test('can prevent addition', () => {
expect(dom.querySelector('span')).toBeFalsy()
})

test('can impact addition', () => {
test('can impact addition', async () => {
let dom = createElement('<div></div>')

morph(dom, '<div><span>foo</span></div>', {
await morph(dom, '<div><span>foo</span></div>', {
added(to) {
to.textContent = 'bar'
}
Expand All @@ -166,10 +166,10 @@ test('can impact addition', () => {
expect(dom.querySelector('span').textContent).toEqual('bar')
})

test('can prevent addition for tag replacement', () => {
test('can prevent addition for tag replacement', async () => {
let dom = createElement('<div><h1>foo</h1></div>')

morph(dom, '<div><span>foo</span></div>', {
await morph(dom, '<div><span>foo</span></div>', {
adding(to, prevent) {
prevent()
}
Expand All @@ -178,10 +178,10 @@ test('can prevent addition for tag replacement', () => {
expect(dom.querySelector('span')).toBeFalsy()
})

test('can impact addition for tag replacement', () => {
test('can impact addition for tag replacement', async () => {
let dom = createElement('<div><h1>foo</h1></div>')

morph(dom, '<div><span>foo</span></div>', {
await morph(dom, '<div><span>foo</span></div>', {
added(to) {
to.textContent = 'bar'
}
Expand Down
28 changes: 14 additions & 14 deletions tests/jest/morph/internal.spec.js
View file
Open in desktop
@@ -1,50 +1,50 @@
let { morph } = require('@alpinejs/morph')
let createElement = require('./createElement.js')

test('changed element is the same element', () => {
test('changed element is the same element', async () => {
let dom = createElement('<div><span>foo</span></div>')

dom.querySelector('span').is_me = true

morph(dom, '<div><span>bar</span></div>')
await morph(dom, '<div><span>bar</span></div>')

expect(dom.querySelector('span').is_me).toBeTruthy()
})

test('non-keyed elements are replaced instead of moved', () => {
test('non-keyed elements are replaced instead of moved', async () => {
Copy link
Contributor Author

@VicGUTT VicGUTT Dec 25, 2021 •
edited

Choose a reason for hiding this comment

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

TODO: Fix this test

  • Figure out the intent
  • Figure out what went wrong where

Copy link
Collaborator

Choose a reason for hiding this comment

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

The intent is to demostrate that the first list item does not move. The test sets a custom property on the li tag then it triggers a mutation where the text content of the first list item changes and a second list item is added. Note, the new dom element looks like the first one before the mutation but it's actually a brand new node as the custom property is still set on the first item. If the node was keyed, the mutation would move the first node to position due and prepend a new node so the custom property would be on the second list item.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh no I did get what the code was doing, my question was more like "Why is this the expected behavior in the first place". But I admit my phrasing wasn't the best, and thanks for the thorough answer ✌️.

Now I figured this behavior is wanted when the lookahead option is set to false, otherwise the expected behavior is the opposite, meaning : "non-keyed elements are moved instead of replaced".

So my question now would go to @calebporzio directly because I'm confused as to what the default boolean value for the lookahead option should be.

The docs sais it should be false but it was changed to true in #d07aeee.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update: I feel confident in thinking the default lookahead value is false, therefore I've pushed my latest changes ✌️

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep, it's already reverted to false on the main branch. 👍

let dom = createElement('<ul><li>bar</li></ul>')

dom.querySelector('li').is_me = true

morph(dom, '<ul><li>foo</li><li>bar</li></ul>')
await morph(dom, '<ul><li>foo</li><li>bar</li></ul>')

expect(dom.querySelector('li:nth-of-type(1)').is_me).toBeTruthy()
})

test('keyed elements are moved instead of replaced', () => {
test('keyed elements are moved instead of replaced', async () => {
let dom = createElement('<ul><li key="2">bar</li></ul>')

dom.querySelector('li').is_me = true

morph(dom, '<ul><li key="1">foo</li><li key="2">bar</li></ul>')
await morph(dom, '<ul><li key="1">foo</li><li key="2">bar</li></ul>')

expect(dom.querySelector('li:nth-of-type(2)').is_me).toBeTruthy()
})

test('elements inserted into a list are properly tracked using lookahead inside updating hook instead of keys', () => {
test('elements inserted into a list are properly tracked using lookahead inside updating hook instead of keys', async () => {
let dom = createElement('<ul><li>bar</li></ul>')

dom.querySelector('li').is_me = true

morph(dom, '<ul><li>foo</li><li>bar</li></ul>', {
await morph(dom, '<ul><li>foo</li><li>bar</li></ul>', {
lookahead: true,
})

expect(dom.outerHTML).toEqual('<ul><li>foo</li><li>bar</li></ul>')
expect(dom.querySelector('li:nth-of-type(2)').is_me).toBeTruthy()
})

test('lookahead still works if comparison elements have keys', () => {
test('lookahead still works if comparison elements have keys', async () => {
let dom = createElement(`<ul>
<li key="bar">bar</li>
<li>hey</li>
Expand All @@ -53,7 +53,7 @@ test('lookahead still works if comparison elements have keys', () => {
dom.querySelector('li:nth-of-type(1)').is_me = true
dom.querySelector('li:nth-of-type(2)').is_me = true

morph(dom, `<ul>
await morph(dom, `<ul>
<li key="foo">foo</li>
<li key="bar">bar</li>
<li>hey</li>
Expand All @@ -67,7 +67,7 @@ test('lookahead still works if comparison elements have keys', () => {
expect(dom.querySelectorAll('li').length).toEqual(3)
})

test('baz', () => {
test('baz', async () => {
let dom = createElement(`<ul>
<li key="bar">bar</li>

Expand All @@ -77,7 +77,7 @@ test('baz', () => {
dom.querySelector('li:nth-of-type(1)').is_me = true
dom.querySelector('li:nth-of-type(2)').is_me = true

morph(dom, `<ul>
await morph(dom, `<ul>
<li>foo</li>

<li key="bar">bar</li>
Expand All @@ -93,15 +93,15 @@ test('baz', () => {
expect(dom.querySelectorAll('li').length).toEqual(3)
})

test('blah blah blah no lookahead', () => {
test('blah blah blah no lookahead', async () => {
let dom = createElement(`<ul>
<li key="bar">bar</li>
<li>hey</li>
</ul>`)

dom.querySelector('li:nth-of-type(1)').is_me = true

morph(dom, `<ul>
await morph(dom, `<ul>
<li key="foo">foo</li>
<li key="bar">bar</li>
<li>hey</li>
Expand Down