Skip to content
This repository has been archived by the owner on Jun 16, 2020. It is now read-only.

update vdom dependency #82

Closed
ahdinosaur opened this issue Sep 4, 2014 · 7 comments
Closed

update vdom dependency #82

ahdinosaur opened this issue Sep 4, 2014 · 7 comments

Comments

@ahdinosaur
Copy link
Contributor

related to Raynos/virtual-hyperscript#8

@mmckegg
Copy link

mmckegg commented Oct 9, 2014

Yes please +1!

There's a bug in the current mercury vdom with diffing attributes that breaks everything (at least for me).

Matt-Esch/vdom@a6c1691

Also @ahdinosaur you should update your pull request to the latest releases (with the bug fixed).

@ahdinosaur
Copy link
Contributor Author

@mmckegg: btw, @pfraze made a new pull request (#91) using the latest vdom for that same bug.

@Raynos
Copy link
Owner

Raynos commented Oct 9, 2014

@mmckegg is that commit broken, or does that commit fix the bug.

the attributes object never worked previously.

@mmckegg
Copy link

mmckegg commented Oct 9, 2014

Sorry, it wasn't attributes that I was after, I got confused by the commit message. The commit also fixed the bug I was facing.

patchObject was missing the declaration of previousValue (see commit before fixed https://github.com/Matt-Esch/vdom/blob/8e0ba4b89257ba4ee2e33df085d857ba78e52db8/apply-properties.js#L47) and was added in the commit I specified above (https://github.com/Matt-Esch/vdom/blob/a6c1691cc4b76d61b7f015ec049627658614fded/apply-properties.js#L49).

Edit: actually now that I'm reading the code properly, it looks like it was attributes that I was after. Sorry for being confusing.

@Raynos
Copy link
Owner

Raynos commented Oct 9, 2014

@mmckegg let me ask it differently.

Is mercury broken or is it missing a feature ?

@mmckegg
Copy link

mmckegg commented Oct 9, 2014

It was missing a feature, but due to a confusing error message, I thought it was a bug.

The old patchObject implementation was incomplete, and would always throw an error when called a second time on the same property.

@Raynos
Copy link
Owner

Raynos commented Nov 9, 2014

I updated vdom.

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

Successfully merging a pull request may close this issue.

3 participants