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

Docs re: component state are both polymorphic and variadic, and that's not good #2293

Closed
CreaturesInUnitards opened this issue Nov 13, 2018 · 6 comments · Fixed by #2294
Closed

Comments

@CreaturesInUnitards
Copy link
Contributor

Premise

Mithril's journey from 0.2.x has been clarifying in dozens, if not hundreds, of ways. Among the starkest of these is with respect to effective approaches to initializing and mutating component state, including via the explicit vnode.state.

I propose that the official documentation should abandon all references to vnode.state, favoring instead the community's hard-won wisdom for each of the three flavors of component.

This is a documentation-only proposal. I'm opening this issue to take the community's temperature.

Expected Behavior

Proper documentation is not a codex of All Which Has Been Made Possible, it's a manual of effective practice, informed by the explicit experience of the battle-tested. Discussions of component state should reflect both of these facts.

Currently...

There are at least nine (9!) ways to initialize and mutate component state:

  1. POJO: attach and mutate properties of vnode.state
  2. POJO: attach custom properties to the component prototype, e.g. { foo: 'bar', view: ... }
  3. POJO: via 'this', but you gotta be careful
  4. Closure: ditto 1
  5. Closure: ditto 2
  6. Closure: ditto 3
  7. Closure: assign and mutate ivars within the closure
  8. Class: ditto 1
  9. Class: ditto 3

I'm sure I've left out at least one or two.

So.

Possible Solution

  1. It has been accepted for some time now that POJOs "should" be stateless. If component state is needed, we achieve the same ends more clearly and simply if we use closures instead. I therefore suggest we omit documentation of any of the 3 approaches to component state wrt POJOs.

  2. It has likewise been widely accepted that instance variables within a closure are the best places to store component state. I therefore suggest we omit documentation of any of the 3 Closure approaches which are duplicates of those also available to POJOs.

  3. If this has a reasonable use case, it's in classes. TBH I'm not convinced, but be that as it may, there's a significant population which appreciates the class variant, and for them we should document component state using this. Finally: I personally don't see a compelling reason to document vnode.state for classes, but others (with actual experience using class components) may disagree.

Justification for Omissions

There are dozens, if not hundreds, of useful patterns and features which we don't — and shouldn't — document, e.g. view functions, unidirectional flow, etc. Fluent developers will figure those things out for themselves, and find ways to mold Mithril to their needs. We don't need to peel back the curtain for them.

But!

If we document features and/or patterns which we have learned from experience are suboptimal, we do the community a disservice. Our documentation should be confident and opinionated enough to pass on our accumulated wisdom under the auspices of How It's Done.

Thoughts?

@spacejack
Copy link
Contributor

I therefore suggest we omit documentation of any of the 3 Closure approaches which are duplicates of those also available to POJOs.

I think I did that with the latest docs PR.

@CreaturesInUnitards
Copy link
Contributor Author

CreaturesInUnitards commented Nov 13, 2018

@spacejack oh, of course — my language is ambiguous there. I meant to say "of the 9 approaches listed, let's leave these out going forward". Not meaning to suggest anything whatsoever about what's actually there.

@highmountaintea
Copy link

My biggest frustration with mithril is how outdated the documentation is. Combing through the internet, you get pulled into 100 different directions, most of them the outdated direction, so your proposal is very welcomed in my opinion. I would say less is better in this instance, so just focus on POJOs and closures, and don't mention this

@spacejack
Copy link
Contributor

For V1-2 I think we should stick with the docs more or less as they are. But we do need to get into more detail for classes and POJOs in particluar.

For classes we should probably point out other options (as React has) of setting up bound methods to be used for callbacks. Either by assigning an arrow function in the constructor or with the class properties proposal.

For POJOs we should add another example that lines up with the closure & class examples. We should cover using vnode.state rather than this and using arrow function wrappers. (There is basically one sentence dedicated to this seemingly as an afterthought.) We should get into a bit more detail about using POJO properties as default initialization (i.e. the problem of using object references rather than primitives.)

These features exist and I think need to be explained. I figure we cover it here once, in depth, then just use closures for stateful examples everywhere else (which is done already, I think...)

There is the other problem of so many examples in the wild using various state mechanisms. But I'm not sure what to do about that.

@barneycarroll
Copy link
Member

@CreaturesInUnitards I think this is an extremely good idea. Vnode state is incredibly complex; there is currently massive conceptual redundancy in the varieties of ways component state can be handled, but the caveats and edge cases surrounding the subtleties of vnode state, this, and vnode access are legion. People can and will be attached to various aspects of various alternatives for various reasons, but the decision making process is obnoxious. Going by familiarity many people are reassured by the OOP appearance of POJOs and classes as stateful entities, but this is often misleading (the virtual DOM paradigm is in many ways at odds with the implications of passed references and objects calling each others methods), and leads to a complex litany of gotchas that detract from the power of Mithril. Moreover, people often make assumptions in comparison to other libraries that implement similar surface APIs that lead to disappointment: vnode state is magical and mysterious, but it's unrelated to redraw logic as it is with Mithril; class arrow methods allow React components to mitigate against the context gotchas of DOM event handler binding, but that won't work for us because attributes aren't accessible via this, and even so the behaviour deviates disastrously when applied to POJO components (whereas they are otherwise functionally nigh identical)… The list grows over time: the practical use cases are all the same but the implementation footguns abound as a font of highly technical consideration that could be better spent on the practical implications of Mithril's various APIs.

@spacejack as a compromise, how about we go for the path of simplicity foremost, but give component syntactic variation a very cursory explanation linked to an in-depth (but inevitably non-exhaustive) treatise on some of the various exotic features and gotchas of vnode state implementation?

@CreaturesInUnitards
Copy link
Contributor Author

CreaturesInUnitards commented Nov 13, 2018

@spacejack ok, I have a PR almost finished which addresses all of this while simultaneously using 93% of #2292. It's mostly a rearrangement, but it treats Classes independently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Completed/Declined
Development

Successfully merging a pull request may close this issue.

4 participants