Swizz/snabbdom-pragma

Destructuration is broken for custom components

Sinewyk opened this issue · 10 comments

Sinewyk commented

Since 9e3efb3 destructuration is broken.

In particular, the jsx spec seem to specify to send null when there are no props.

You were handling that case before this refactoring 9e3efb3#diff-1fdf421c05c1140f6d71444ea2b27638L29

And after 9e3efb3#diff-1fdf421c05c1140f6d71444ea2b27638R48 you just rely on the default argument behavior of es6 ... which is equivalent to === void 0 or === undefined.

Which breaks for null of course. So when there are no props, you send null instead of {} downstream, and when destructuring it breaks down.

You should manually check for null as you previously did, it will never be undefined here as the jsx spec specifies ^^.

Sinewyk commented

As a sidenote: I think tons of issues are going to disappear.

Right now if I go from <p className="foo">Foo</p> to <p>Bar</p> then the className is not deleted.

Is suspect because it receives null instead of {} ...

Sinewyk commented

Forget it. You do send {} in case of null and you have the tests to verify it, so why do I get destructuration issues 😥

Sinewyk commented

Ok I get it. You don't sanitize props if the sel is a function (custom components). So doing something like

function MyCustomStuff({ someProp }) {
  return <div someProp/>;
}

But using it like this

<MyCustomStuff/>
// which desugars to
h(MyCustomStuff, null, []);
// instead of 
// h(MyCustomStuff, {}, []);

breaks down. Even if we use default arguments in the component implementation (because null !== undefined).

Should we consider this a bug ? Or should we also sanitize props when using custom components ?

Sinewyk commented

Would you accept a PR @Swizz or ?

ryota-ka commented

Any progress on this?

Swizz commented

Hi,
I have no ETA on this.
TBH I am not using Snabbdom anymore as I am now a contributor/member of Hyperapp.

Maybe, I will proceed with PR only at the moment.

blikblum commented

If anyone is interested i published a package with the fix as snabbdom-pragma-lite.

With bonus you get 30% smaller code, no dependency on extend and 10x faster in real world usage

Swizz commented

In case some of you are interested in this, I am open of transferring ownership or adding you as a contributor, as you wish.

blikblum commented

I would be happy to take the duty of maintaining it