search

jorgebucaran / superfine

Absolutely minimal view layer for building web interfaces
https://git.io/super
MIT License
1.57k stars 78 forks source link

Flow? #10

Closed jorgebucaran closed 7 years ago

jorgebucaran commented 7 years ago

Should we use flow?

zaceno commented 7 years ago

For myself (who has never used typescript or flow in anything, so I shouldn't really be talking... :-P ) I don't really see the point of adding a type checker to this, as it:

but at the same time It wouldn't do any harm (I think?) So if adding Flow would give you that good feeling of clean, polished code, then go for it :)

zaceno commented 7 years ago

Ah... I see now that we did not bring the hyperapp tests over yet. We should do that. I also noticed there's already an issue about that. Once we settle on a test framework, I'm happy to help out porting the tests (time permitting)

sumeetsarkar commented 7 years ago

Hi, I think we do not need to invest a lot for Flow, instead, for a small lib like picodom (which is already readable), let's use the comment based syntax of Flow for type checking.

jorgebucaran commented 7 years ago

@sumeetsarkar I didn't know about this. Thanks for suggesting it.

leo-shopify commented 7 years ago

For what is worth, typescript also supports comment syntax, and since it uses standard jsDoc syntax it operates with other tools that support it.

jorgebucaran commented 7 years ago

@andyrj Do you want to take this one?

andyrj commented 7 years ago

Sure, I will start a PR with basic flow typing. I will need help with typescript, haven't dug into it like I have flow.

jorgebucaran commented 7 years ago

@andyrj Can we do typescript on a different PR? I was hoping you'd only take care of the flow file. Like you are trying to do in hyperapp.

andyrj commented 7 years ago

@jbucaran ya I am still getting my coffee in me, just saw that typescript pr, I was only going to do flow :smile:

andyrj commented 7 years ago

I made a PR for flow library definition, did you want type comments in the code base itself like @sumeetsarkar mentioned above?

andyrj commented 7 years ago

I am going to change my flow PR, to generate the index.js.flow in the build from type declarations added to the source so that it is distributed with npm. https://medium.com/@ryyppy/shipping-flowtype-definitions-in-npm-packages-c987917efb65

andyrj commented 7 years ago

My PR has an initial hand written picodom.js.flow, that would need to be copied into dist/ on build, and then I went through all the necessary steps to add flow to the code base itself in later commits of that PR. I tend to feel like adding flow to the code base is marginal, as it ends up requiring some helper functions to work around flow issues. (Invariant, and getIndexedElement in my PR)

I think we should just hand craft a picodom.js.flow and copy it into dist/, which wouldn't require any changes to the code and would allow users to utilize flow.

Curious what your guys take is however which is why I went ahead and made the code changes that would be needed in that PR.

jorgebucaran commented 7 years ago

Closing in favor of #24! 😄