choojs / nanocomponent

🚃 - create performant HTML components
https://leaflet.choo.io/
MIT License
366 stars 30 forks source link

API method names #86

Closed willurmston closed 5 years ago

willurmston commented 5 years ago

Hey there!

I am coming to the Choo universe (chooniverse?) from React and so far I like the lighter-weight approach. There were some confusing things about the Nanocomponent API that held me up a bit. I'm curious about the reasoning behind the API method names.

  1. Why isn't update() called something like shouldUpdate()? As a user, I would expect the component to re-render if I call this.update()
  2. Why are the lifecycle methods all lowercase, but don't use the on prefix? Is there a precedent for this? Same issue as above, where I would expect load() and unload() to do something different. Maybe it would make more sense to use mount / unmount since those terms are used elsewhere in the Choo documentation?

These questions may have simple answers but they weren't obvious to me. Thanks for all your work!

marlun commented 5 years ago

I think at least part of the reason is that they try to adhere to the web standards which uses all lowercase like el.onclick = fn

https://developer.mozilla.org/en-US/docs/Web/Events

willurmston commented 5 years ago

I'm on board with that, but why not go all the way and include the on prefix?

bcomnes commented 5 years ago

I think it may have at one point. Naming things is difficult😩

yoshuawuyts commented 5 years ago

I'm on board with that, but why not go all the way and include the on prefix?

That's a great question. The naming of components has been a 2 year process, but if I recall correctly. I believe at some point we were experimenting with components we also tried a style that was similar to:

var component = Component('my-component')
component.on('mount', () => console.log('mounted'))

Which we then quite literally translated into "what if we removed the .on() part, and made it method names instead", and ended up with names like mount () {} instead.

We also went through a brief phase where the methods that needed to be implemented were prefixed with underscores, and I think most of the people in the community were happy enough we settled on reasonably descriptive, brief methods without underscores.

I hope this provides an okay historical anecdote for how we got to the naming we currently have, haha.

willurmston commented 5 years ago

Thanks for the detailed explanation! I can understand your reluctance to change the API again. Maybe one day 🚂🌈

blahah commented 5 years ago

Late to the party but personally I find the all-lowercase style and removal of unclear prepositions to be really helpful.

The only outlier is createElement - this uses camelCase while afterupdate and afterreorder are all lowercase. Is this an oversight or something deliberate? If an oversight, is it open to being changed or does API stability at this point carry more weight than API predictibility? I can see arguments in both directions.

ungoldman commented 5 years ago

Hi @blahah -- I believe generally the idea was to follow existing conventions from the DOM spec. So createElement comes from document.createElement()

blahah commented 5 years ago

Very sensible. So it seems my only recourse is to go back in time and prevent capital letters entering the written language.

ungoldman commented 5 years ago

Going to close this since changing method names in a future major release isn't on the roadmap for now. Open to making this a priority if enough people feel it's important.

Putting this issue to bed for now 🛌😴