Fix failing tests - account for the morph function now returning a promise #2528
Conversation
|
|
||
| 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 () => { |
There was a problem hiding this comment.
TODO: Fix this test
- Figure out the intent
- Figure out what went wrong where
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Update: I feel confident in thinking the default lookahead value is false, therefore I've pushed my latest changes ✌️
There was a problem hiding this comment.
Yep, it's already reverted to false on the main branch. 👍
packages/morph/src/morph.js
Outdated
| @@ -319,6 +319,8 @@ function addNodeTo(node, parent) { | |||
|
|
|||
| return clone | |||
| } | |||
|
|
|||
| return parent; | |||
There was a problem hiding this comment.
I'm not sure it should return parent since that one is not an added node and it would make the debugger print thr wrong statement. It should probably return null or another falsy value and the caller should handle it properly.
There was a problem hiding this comment.
Yes, agreed, good catch. I've returned null from the function instead of parent and made the following changes to line 198 :
- let added = addNodeTo(currentTo, from)
+ let added = addNodeTo(currentTo, from) || {}The variable added is only used for the debugger; and I'm assuming having a message of Add element: undefined is tolerable.
|
@SimoTod Thanks for the feedback. I have definitely not forgotten my PRs. This new year started with lot of life changing events on my end. That said, I should be back on this somewhere this week or at most early next week ✌️ |
|
Thanks! |
The morph tests do not account for the morph function now being asynchronous. This PR is an attempt to remedy that.
That said, this PR seemed to have revealed a false positive in the test
non-keyed elements are replaced instead of moved. I'll try and figure out what's going on at a later date.