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

Add type properties to hooks #247

Closed
wants to merge 1 commit into from
Closed

Conversation

tcoats
Copy link

@tcoats tcoats commented May 16, 2015

AttributeHook has a type property available on it's prototype but the other hooks don't. Providing the type property helps other libraries parse vdom, such as this discussion in vdom-to-html. At the moment there is no easy way to convert a hook to html. Using instanceof requires the same npm package which isn't always possible.

This PR adds a type property to ev-hook, focus-hook and soft-set-hook.

If it makes sense it might also be nice to add corresponding is-ev-hook, is-focus-hook and is-soft-set-hook files in vnode.

Thoughts?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 97.5% when pulling 4653975 on odojs:hook-to-html into 88534bb on Matt-Esch:master.

@kuraga
Copy link
Contributor

kuraga commented May 16, 2015

See also #248

@DeTeam
Copy link

DeTeam commented Jun 23, 2015

@tcoats what do you think about having .toString on hook's prototype?

@TimBeyer
Copy link

I think @DeTeam has the better proposal here.
That way html conversion behavior is defined by whoever defines a hook and elegantly solved by polymorphism instead of starting to switch on some kind of type identifier.
This would solve the issue completely for the future.

@tcoats
Copy link
Author

tcoats commented Jun 23, 2015

@DeTeam I think it's great!

The only drawback is that each component will need to include a stringify library - but the freedom string gives over vdom is excellent.

@DeTeam
Copy link

DeTeam commented Jun 24, 2015

The only drawback is that each component will need to include a stringify library

@tcoats, not really :) it's all about being able to represent Hook as a string. vdom-to-html lib can't guess on how to do it. If we go with just using hook.value it doesn't fix user hooks, while having something like toString, render as part of Hook interface will allow custom hooks to be automatically handled by vdom-to-html.

@tcoats
Copy link
Author

tcoats commented Jun 24, 2015

@DeTeam In my case I'll probably need to use a stringify lib more if toString is added to widgets as well, which I think is a great idea. Should toString be added to widgets?

toString is an excellent and useful addition!

@tcoats tcoats closed this Aug 22, 2023
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.

5 participants