Swizz / snabbdom-pragma

'NotReact.createElement' jsx pragma although for snabbdom
MIT License
47 stars 16 forks source link

VNodes in children become undefined after createElement() #16

Open wbern opened 7 years ago

wbern commented 7 years ago

I have this virtual node (called children), previously created using createElement()

[{
    "sel": "button",
    "data": {
        "props": {
            "type": "button"
        },
        "on": {}
    },
    "children": [{
        "text": "Remove"
    }]
}]

I want to pass this into a new createElement()-call like below:

Snabbdom.createElement('div', undefined, children);

But the result then becomes:

{
    "sel": "span",
    "data": {},
    "children": [null]
}

If I however do Snabbdom.createElement('div', undefined, h('div', undefined, children));, I get the correct output (but with an additional unwanted element inbetween):

{
    "sel": "span",
    "data": {},
    "children": [{
        "sel": "div",
        "children": [{
            "sel": "button",
            "data": {
                "props": {
                    "type": "button"
                },
                "on": {}
            },
            "children": [{
                "text": "Remove"
            }]
        }]
    }]
}

Is there a better work-around to re-use vnodes? Would it be possible to allow vnodes to be passed into children, like what h()-calls currently allow?

Swizz commented 7 years ago

This is a weird issue given the following test pass.

Could you provide me a gist of your code or simplified one for testing purpose ?

wbern commented 7 years ago

Ok, I figured out why this is happening. It's because the undefined properties have gone away at some point in the object, namely elm, key and text.

After doing the below in my case, I started getting the correct outcome.

children[0].elm = undefined
children[0].key = undefined
children[0].text = undefined

I suspect the properties are going away because I am doing some modifications before/after the createElement()-calls, so I won't hold this repo accountable for that.

Swizz commented 7 years ago

Snabbdom-pragma main use is to move from moving to JSX pragma call to a Snabbdom Vnode, majors transformations are to handle the React.createElement api for creating a Vnode following the Snabbdom needs.

Each Vnode we work with are trusted Snabbdom Vnode, so I never figured out of outly manipuled Vnodes.

wbern commented 7 years ago

Because Snabbdom does not deal with component lifecycles in a more react-like manner, I decided to do some sort of "pre-processing" to custom components. The only thing that remains in my code is the sel-property, along with the children and data property.

Swizz commented 7 years ago

Snabbdom-pragma aim to be compiler/transpiler and framework independent focusing only on NotReact.createElement() api and returning a Snabbdom Vnode. To be used everywhere a JSX to createElement transformation is possible.

Im glad you found a solution on your own.

wbern commented 7 years ago

So I figured out exactly the reason for why elm and other properties are missing, is because the map() function in javascript does not keep properties that are undefined in the final result.

so image

after a .map(val => val) call becomes

[{
    "": {
        "sel": "span",
        "data": {},
        "children": [{
            "text": "a test"
        }]
    }
}]

For me personally, this could be a reason for covering these missing properties in snabbdom-pragma. the snabbdom/h()-call seems to address it.

Swizz commented 7 years ago

So : Not defined Vnode properties need to be handle or set as undefined.

wbern commented 7 years ago

This is a 1:1 copy of my wrapper function, where I recurse into the children and set them to undefined if they don't exist. This will make null turn into undefined as well due to the || operator, but null assigned properties don't work very well right now anyway throughout the Snabbdom implementation as I've noticed in the past.

export function createElement(ComponentOrTag, models, ...children) {
    // quirk: because functions like map() like to drop undefined properties,
    // put them back into the vnodes, so Snabbdom.createElement doesn't fail
    const recurseDefineProperties = (vnode) => {
        if (vnode) {
            for (let i = 0; i < vnode.length; i++) {
                if (vnode[i].sel) {
                    vnode[i].elm = vnode[i].elm || undefined;
                    vnode[i].key = vnode[i].key || undefined;
                    vnode[i].text = vnode[i].text || undefined;
                }

                if (vnode[i].children) {
                    recurseDefineProperties(vnode[i].children);
                }
            }
        }
    };
    recurseDefineProperties(children);

    // If this is a custom component, leave it simply with its name for later processing in sweact
    if (typeof ComponentOrTag === "function") {
        let unresolvedVNode = Snabbdom.createElement(ComponentOrTag.name, models, children);
        unresolvedVNode.cRef = ComponentOrTag;
        return unresolvedVNode;
    }

    return Snabbdom.createElement(ComponentOrTag, models, children);
}
Swizz commented 7 years ago

I am pretty busy, I will work on it as soon as possible.

blikblum commented 6 years ago

@wbern Can you provide the exact code that is mutating the vnode (removing the undefined keys)?

I tried the following but is keeping the properties:

var x = [{key: undefined}]
var y = x.map(val => val) // [{key: undefined}]

I have some ideas how to fix it but need a reproduction example to create a test

wbern commented 6 years ago

@blikblum Sorry I have moved on to another project and don't have access to the material anymore.

ftaiolivista commented 5 years ago

@blikblum you can find sample code here. I think it's about the same problem. Sorry about the non minimal sample.