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

Bug - Morph text nodes. #2524

Merged
merged 3 commits into from Jan 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
42 changes: 21 additions & 21 deletions packages/morph/src/morph.js
Expand Up @@ -6,23 +6,23 @@ function breakpoint(message) {
if (! debug) return

message && logger(message.replace('\n', '\\n'))

return new Promise(resolve => resolveStep = () => resolve())
}

export async function morph(from, toHtml, options) {
assignOptions(options)

let toEl = createElement(toHtml)

// If there is no x-data on the element we're morphing,
// let's seed it with the outer Alpine scope on the page.
if (window.Alpine && ! from._x_dataStack) {
toEl._x_dataStack = window.Alpine.closestDataStack(from)

toEl._x_dataStack && window.Alpine.clone(from, toEl)
}

await breakpoint()

patch(from, toEl)
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 || true
debug = options.debug || false
}

Expand All @@ -71,12 +71,12 @@ async function patch(from, to) {
// don't see a way to enable it currently:
//
// if (from.isEqualNode(to)) return

if (differentElementNamesTypesOrKeys(from, to)) {
let result = patchElement(from, to)

await breakpoint('Swap elements')

return result
}

Expand All @@ -89,7 +89,7 @@ async function patch(from, to) {
if (textOrComment(to)) {
await patchNodeValue(from, to)
updated(from, to)

return
}

Expand Down Expand Up @@ -152,7 +152,7 @@ async function patchAttributes(from, to) {

if (! to.hasAttribute(name)) {
from.removeAttribute(name)

await breakpoint('Remove attribute')
}
}
Expand Down Expand Up @@ -212,7 +212,7 @@ async function patchChildren(from, to) {
currentFrom = addNodeBefore(currentTo, currentFrom)

domKey = getKey(currentFrom)

await breakpoint('Move element (lookahead)')
}
}
Expand All @@ -222,8 +222,8 @@ async function patchChildren(from, to) {
domKeyHoldovers[domKey] = currentFrom
currentFrom = addNodeBefore(currentTo, currentFrom)
domKeyHoldovers[domKey].remove()
currentFrom = dom(currentFrom).nodes.next()
currentTo = dom(currentTo).nodes.next()
currentFrom = dom(currentFrom).nodes().next()
currentTo = dom(currentTo).nodes().next()
Comment on lines +225 to +226
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not relevant to this bug but I believe it's a typo as nodes should be a function call

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good call, should be yeah


await breakpoint('No "to" key')

Expand All @@ -233,7 +233,7 @@ async function patchChildren(from, to) {
if (toKey && ! domKey) {
if (domKeyDomNodeMap[toKey]) {
currentFrom = dom(currentFrom).replace(domKeyDomNodeMap[toKey])

await breakpoint('No "from" key')
}
}
Expand All @@ -244,17 +244,17 @@ async function patchChildren(from, to) {

if (domKeyNode) {
currentFrom = dom(currentFrom).replace(domKeyNode)

await breakpoint('Move "from" key')
} else {
domKeyHoldovers[domKey] = currentFrom
currentFrom = addNodeBefore(currentTo, currentFrom)
domKeyHoldovers[domKey].remove()
currentFrom = dom(currentFrom).next()
currentTo = dom(currentTo).next()

await breakpoint('I dont even know what this does')

continue
}
}
Expand All @@ -263,8 +263,8 @@ async function patchChildren(from, to) {
// Patch elements
await patch(currentFrom, currentTo)

currentTo = currentTo && dom(currentTo).next()
currentFrom = currentFrom && dom(currentFrom).next()
currentTo = currentTo && dom(currentTo).nodes().next()
currentFrom = currentFrom && dom(currentFrom).nodes().next()
Comment on lines +266 to +267
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Relevant change. When getting the next element for the next loop iteration, we want to use nodes() so we don’t skip text nodes.

}

// Cleanup extra froms
Expand Down Expand Up @@ -367,7 +367,7 @@ class DomManager {
this.traversals = {
'first': 'firstChild',
'next': 'nextSibling',
'parent': 'parentNode',
'parent': 'parentNode',
}; return this
}

Expand All @@ -386,7 +386,7 @@ class DomManager {
replace(replacement) {
this.el[this.traversals['parent']].replaceChild(replacement, this.el); return replacement
}

append(appendee) {
this.el.appendChild(appendee); return appendee
}
Expand Down
11 changes: 10 additions & 1 deletion tests/cypress/integration/plugins/morph.spec.js
@@ -1,4 +1,4 @@
import { haveText, html, test } from '../../utils'
import { haveText, haveHtml, html, test } from '../../utils'

test('can morph components and preserve Alpine state',
[html`
Expand Down Expand Up @@ -133,3 +133,12 @@ test('can morph teleports',
get('h2').should(haveText('there'))
},
)

test('can morph text nodes',
[html`<h2>Foo <br> Bar</h2>`],
({ get }, reload, window, document) => {
let toHtml = html`<h2>Foo <br> Baz</h2>`
get('h2').then(([el]) => window.Alpine.morph(el, toHtml))
get('h2').should(haveHtml('Foo <br> Baz'))
},
)
2 changes: 2 additions & 0 deletions tests/cypress/utils.js
Expand Up @@ -95,6 +95,8 @@ export let haveText = text => el => expect(el).to.have.text(text)

export let notHaveText = text => el => expect(el).not.to.have.text(text)

export let haveHtml = html => el => expect(el).to.have.html(html)

export let beChecked = () => el => expect(el).to.be.checked

export let notBeChecked = () => el => expect(el).not.to.be.checked
Expand Down