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

merge features from virtual-hyperscript #114

Closed
wants to merge 2 commits into from
Closed

Conversation

Matt-Esch
Copy link
Owner

I went to update virtual-dom to the latest version of virtual-hyperscript, but it depends on vtree. In my efforts to reduce this "yet another vtree dependency" problem, I have simply merged the changes from virtual-hyperscript into virtual-dom/h. cc @Raynos

@coveralls
Copy link

Coverage Status

Coverage decreased (-7.68%) when pulling bb7f1ee on virtual-hyperscript into 8577b4f on master.

@Raynos
Copy link
Collaborator

Raynos commented Nov 9, 2014

@Matt-Esch

Rather then copying and pasting the source code of virtual-hyperscript in here. It might be better to just delete the h code completely.

Alternatively having another copy of vtree instead would be preferable over copy pasted code.

If you really do want to copy paste, consider using a git submodule or something.

@gsf
Copy link
Contributor

gsf commented Nov 10, 2014

Copying and pasting? Submodules? Ugh.

If h is only used for the tests, then virtual-hyperscript should be a devDependency of virtual-dom. If virtual-dom, virtual-hyperscript, and vdom all need to rely on the same instance of vtree, then all three should have vtree as a peerDependency.

@gsf
Copy link
Contributor

gsf commented Nov 10, 2014

...and I'm catching up with the last year of hate on peerDependencies (npm/npm#5080). If we're tempted in that direction is that a sign that we could be handling things better? Do virtual-dom, virtual-hyperscript, and vdom (and mercury) all need to depend on vtree, or could these dependencies just be at the API level (akin to Mikeal's suggestion)?

@Raynos
Copy link
Collaborator

Raynos commented Nov 10, 2014

@gsf peer dependencies are the wrong thing.

There are complex dependencies on the data structures in vtree.

We just have to manage the versions and not fuck it up. That's what we get for having complicated data structures.

Also the fact that there are tests in virtual-dom instead of there being tests in vdom and vtree is weird. If we need h for tests we should just delete the tests and move them to vtree and vdom.

@gsf
Copy link
Contributor

gsf commented Nov 10, 2014

@Raynos Agreed on all points (especially moving tests out of virtual-dom).

@Matt-Esch
Copy link
Owner Author

If you take mikeals suggestion, I would go around turning all of the things that depend on vtree or vdom into interfaces that allow you to initialize that dependency with a version of vtree/vdom. Note that in this case, vdom becomes module.exports = function(vtree), as does virtual-hyperscript. vdom should probably also expose the vtree things because having function(vdom, vtree) introduces yet another point of inconsistency.

I was intending for virtual-dom to be a consistent easy to use export. The MVP without causing version suffering. The tests in virtual-dom are integration tests, proving that these modules work together at these versions. The vdom and vtree modules should have separate unit tests. Technically, given the "inverted dependency" vdom has on vtree, I should be mocking the vtree dependency in the vdom tests. It's a mess, I'm open to suggestions that don't come with the caveat of version hell. Clearly what we are doing right now is bad.

We just have to manage the versions and not fuck it up.

^ This is bs. "We" becomes everyone who uses these modules.

@Raynos
Copy link
Collaborator

Raynos commented Nov 12, 2014

That is not mikeals suggestion. You depend on interfaces not functions.

Since one of the advantages of seperate modules is less KBs in the browser, we can achieve this by just having the virtual-dom package expose multiple modules.

The current situation is bad, might as well move them all back into virtual-dom.

@Matt-Esch
Copy link
Owner Author

Well, looks like we moved everything. Also @Raynos what you depend on and how you depend on it are 2 different things. It seems Mikeal is suggesting that you depend on an interface, and the concrete implementation is supplied to that depending module instead of it providing for itself via require.

@Matt-Esch Matt-Esch closed this Nov 18, 2014
@Matt-Esch Matt-Esch deleted the virtual-hyperscript branch December 28, 2014 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants