Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Proposal to move to move to Flow types #1260

Closed
8 tasks
vmx opened this issue Mar 13, 2018 · 12 comments
Closed
8 tasks

Proposal to move to move to Flow types #1260

vmx opened this issue Mar 13, 2018 · 12 comments
Assignees
Labels
status/ready Ready to be worked

Comments

@vmx
Copy link
Member

vmx commented Mar 13, 2018

Flow types will be introduced into the js-ipfs stack. We'll start with some smaller libraries and see how it goes. The goal is that it doesn't break existing things and the libraries can be used as before. If we find problems with that APIs due to adding types, those of course will be fixed.

If a new workflow is needed, it will be documented. Starting to contribute to a js-ipfs project shouldn't be any harder than it is now.

The tooling will change a bit. We introduce Prettier for automated code formatting as based on @Gozala's experience standardJS doesn't play that nice with Flow types. Though we will use the prettier-standard version so that the existing code base doesn't need to be changed too much and people used to coding in standardJS won't see their code reformatted too heavily.

The npm commands like test, lint, release are expected to work as usual. They might not use AEgir from the start, but eventually the tools/code needed will be merged into AEgir.

Let's stick to the original plan and get js-cid Flow types. In order to achieve that, port all the multiformats projects before that:

This awesome endeavor is lead by @vmx.

References:

@olizilla
Copy link
Member

olizilla commented Mar 13, 2018

I'd like to get my hands dirty on this. Would anyone with experience of flow be up for running a workshop / walkthrough of converting one of the libs listed? Or, lower set up cost... perhaps folk who are already comfortable using flow could buddy up with someone to pair (via zoom, or whatever works) on converting one of the listed libs.

I'd like to have a go at js-multibase if it's not already being worked on.

@Gozala
Copy link
Contributor

Gozala commented Mar 13, 2018

@olizilla I'm happy to help as long as we can work out time that will work with my schedule.

@daviddias
Copy link
Member

@vmx thank you so much for taking the lead on pushing this endeavor forward and @Gozala for starting it in the first place ❤️.

@olizilla I love that idea! Remote Pair Programming has worked really well in the past and I'm down to try it again. I feel that session should be js-multibase + js-cid and if everyone is comfortable, we should record it so that we can use it as an educational tool.

1 hour (including questions) should be enough, right? Wanna do Friday at 5pm UTC?

@Gozala
Copy link
Contributor

Gozala commented Mar 14, 2018

Friday time would work for me. Unfortunately I won't have much time to prepare, if you have specific things you'd like me to cover knowing them ahead would help as well.

Can anyone send me the link to zoom meeting if that's what we going to use.

@daviddias daviddias added the status/ready Ready to be worked label Mar 19, 2018
@vmx vmx mentioned this issue Mar 26, 2018
@garrensmith
Copy link

Would it be worth adding in flow support to aegir, that way we won't need to do so much project setup for each repo?

@vmx
Copy link
Member Author

vmx commented Mar 29, 2018

@garrensmith The plan is to have it in AEgir at one point. I thought we need to figure out how to exactly do things first, to then put it into AEgir. The problem is also that AEgir would still need to work with non-flow projects.

But if you think it's worth it putting it directly into AEgir right now, that would be perfect and I'm happy to help. It could even be on a branch of AEgir for now.

@garrensmith
Copy link

I think it might be nice to implement it in AEgir once multiformats/js-multihash#47 is merged in. That should give us a nice base in terms of how we want flow to work. I think implementing it on a branch as a first attempt would be a good start.

The reason I bring this us is I was looking at adding flow to multihashing, add I'm doing fair bit of copy and pasting initially just to get set up. It's not major but its always nice to avoid it if we can.

But saying all of that I would say having it in AEgir is probably more of a nice to have than something that would be critical.

@fahrradflucht
Copy link

I could help with this endeavour but I'm still unclear on what the plan looks like.

multiformats/js-multihash#47 introduces a build step. While this could bring good things with it the struggle in the past sounds horrible.

I can't find a clear decision on building vs using comments style flow which is what the datastore projects seem to be using.

@vmx
Copy link
Member Author

vmx commented Apr 7, 2018

@fahrradflucht The current plan is:

  • Have this additional build step
  • Don't use prettier, instead adapt the standardjs liniting to make it work

Current PRs that are work in progress on adding types are:

I try to make sure that AEgir works with all this. The current work happend on the flow branch.

So feel free to review those PRs or pick one of those modules mentioned on the original and have a try with adding types. Any help is appreciated, especially comments if we should do things differently based on your experience.

@fahrradflucht
Copy link

@vmx alright, thanks for the clarifications 😃.

Then I will start working on multiformats/js-multibase.

BTW: I noticed that there is no PR for the flow branch on AEgir. Maybe that would a good place for the tooling setup discussion to happen. It is kind of all over the place at the moment.

@vmx
Copy link
Member Author

vmx commented Apr 8, 2018

@fahrradflucht: Good point, I've created a PR on AEgir: ipfs/aegir#213

@vmx
Copy link
Member Author

vmx commented Jun 12, 2020

As much as I love Flow, it really makes sense to move on t o TypeScript definitions, which is tracked here: #2945. Though it's still the same idea that we use pure JS and add definitions in comments.

@vmx vmx closed this as completed Jun 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status/ready Ready to be worked
Projects
None yet
Development

No branches or pull requests

6 participants