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

VNodes in children become undefined after createElement() #16

Open
wbern opened this issue Jun 7, 2017 · 12 comments
Open

VNodes in children become undefined after createElement() #16

wbern opened this issue Jun 7, 2017 · 12 comments
Labels

Comments

@wbern
Copy link

wbern commented Jun 7, 2017

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?

@wbern wbern changed the title VNodes in children becomes undefined after createElement() VNodes in children become undefined after createElement() Jun 7, 2017
@Swizz
Copy link
Owner

Swizz commented Jun 7, 2017

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
Copy link
Author

wbern commented Jun 7, 2017

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
Copy link
Owner

Swizz commented Jun 7, 2017

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
Copy link
Author

wbern commented Jun 7, 2017

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
Copy link
Owner

Swizz commented Jun 7, 2017

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
Copy link
Author

wbern commented Jun 7, 2017

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
Copy link
Owner

Swizz commented Jun 7, 2017

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

@Swizz Swizz added the bug label Jun 7, 2017
@wbern
Copy link
Author

wbern commented Jun 7, 2017

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
Copy link
Owner

Swizz commented Jun 12, 2017

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

@blikblum
Copy link
Collaborator

@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
Copy link
Author

wbern commented Jun 4, 2018

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

@ftaiolivista
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants