Skip to content
This repository has been archived by the owner on Mar 23, 2023. It is now read-only.

Add explanation for flow generic types in documentation #390

Open
VynceMontgomery opened this issue Jan 19, 2017 · 5 comments
Open

Add explanation for flow generic types in documentation #390

VynceMontgomery opened this issue Jan 19, 2017 · 5 comments

Comments

@VynceMontgomery
Copy link

https://github.com/facebook/flux/blob/master/docs/Flux-Utils.md has a capital T show up often in the section on ReduceStore but I see no explanation of what it is supposed to mean. Is this copy-pasta of markup from a previous version of the docs? Or is it supposed to mean something?

@kyldvs
Copy link
Contributor

kyldvs commented Jan 19, 2017

Good point I can see how that would be confusing. T is the polymorphic type specified in the ReduceStore class: https://github.com/facebook/flux/blob/master/src/stores/FluxReduceStore.js#L42

Similar to polymorphic types/generics in many other languages.

Do you have any recommendations for how to capture that more clearly in the docs?

@VynceMontgomery
Copy link
Author

Wow. OK, it hadn't occurred to me that "T" was a proper name, and that it was supposed to be conveying meaningful content on its own.

I mean, the usual thing is to link to an explanatory text or document when a novel term is first used; that seems like it might apply here, if there's a useful thing you can link to. Personally, I would advocate for a longer and more descriptive name, especially since "T" is incredibly unfriendly to web searches, and the convention method(params): returnValue is obscure enough that it isn't even obvious that's what you're doing. You could also spell out what it means, and what that means, the first time you use it:

**getState(): T** Getter method that returns the entire state of this store (a TState object, as defined [TState|here]. This is a thing that is different from the store itself and is useful if you know what to do with it because blah blah blah.) If your state is not immutable you should override this and not expose state directly.

That said, I don't see any more useful documentation at the provided link, which appears to just be an inline version of approximately what's here; it doesn't actually explain what a TState is, or why you're calling it that instead of saying that this returns the store. In fact, "TState" doesn't appear to be very search-friendly, either (though it is a bit better than just "T") in terms of finding out what this means or does. I suspect it may not be a basic enough concept that it's safe to just assume everybody already has it. Or maybe I'm just not the target audience for this documentation, and the people it's for will understand it. But, if understanding it is key to grokking flux, it may be worth finding and pointing to -- or writing and pointing to -- a clear explanation.

@kyldvs
Copy link
Contributor

kyldvs commented Feb 23, 2017

Thanks for the perspective, everything you're saying makes sense and I expect if you had issues understanding what T means others will as well. I'll try to add documentation with links to an explanation of polymorphic types in flow, or a quick example or something in the docs to explain what it means.

@kyldvs kyldvs changed the title Confusing style convention in docs for flux-utils? Add explanation for flow generic types in documentation Aug 4, 2017
@spacedarcy
Copy link

+1 to this. Context: I inherited a Flux project that was created without the use of Flow. I'm now going back through the docs with the intent of tightening up our implementation, since it's started to wander away from the original paradigm. However, when I looked through the docs earlier today I saw the Flow markup and got confused; is Flow a prerequisite for using the flux-utils? Will this work with my ES6 project? Who can I even ask about this, since the default response of my peers is "switch to Redux"?

@kyldvs
Copy link
Contributor

kyldvs commented Mar 28, 2018

Flow is not required and simply additional. The todo-mvc example does not use it: https://github.com/facebook/flux/blob/master/examples/flux-todomvc/src/data/TodoStore.js

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

No branches or pull requests

3 participants