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

Distribute flow types #40

Merged
merged 1 commit into from Oct 1, 2018
Merged

Distribute flow types #40

merged 1 commit into from Oct 1, 2018

Conversation

TrySound
Copy link
Contributor

This simple trick works similary to bundling. One file reexports all
sources with its types.

// @flow

export * from '../src';

@bvaughn
Copy link
Owner

bvaughn commented Aug 11, 2018

I don't want to ship the entire source in the NPM bundle.

@bvaughn
Copy link
Owner

bvaughn commented Aug 11, 2018

I appreciate the suggestion though. A similar suggestion on react-flame-graph prompted my comment earlier on Twitter. I don't find shipping the entire source to NPM a very satisfactory solution.

@TrySound
Copy link
Contributor Author

This is the easiest solution. flow-copy-source is the same publishing sources. NPM module bloat happens when authors publish tests, fixtures, dozens of configs. Sources of this project is just 7 not too big files. There won't be bloat.

Any generation doesn't work in flow well. flow-copy-source makes flow more verbose when types are out of sync with sources and previously built types have an error. So this is just the least hacky solution. I use it in a lot of projects and it just works without hacks and broken packages.

https://unpkg.com/react-beautiful-dnd@9.0.0/dist/
https://unpkg.com/react-powerplug@1.0.0-rc.1/dist/
https://unpkg.com/react-select@2.0.0/lib/
https://unpkg.com/jss@9.8.7/lib/
https://unpkg.com/react-head@3.0.0-0/dist/

@TrySound
Copy link
Contributor Author

@bvaughn Is it worth to bikeshed about distribution and dependency bloat when this library is quite small. Sources inside node_modules won't make anybody's node_modules Bigger.

Can we go this way until we will find a better solution?

@bvaughn
Copy link
Owner

bvaughn commented Sep 28, 2018

Sources inside node_modules won't make anybody's node_modules Bigger.

Isn't that exactly what putting sources in node_modules does?

I dunno. I still really dislike this, and I'm upset with Flow for not providing a better solution for such an obvious use case.

I'll think about it some more.

@TrySound
Copy link
Contributor Author

I meant sources of this library specifically. They are quite small.

@bvaughn
Copy link
Owner

bvaughn commented Sep 28, 2018

Makes sense 😄

This simple trick works similary to bundling. One file reexports all
sources with its types.

```js
// @flow

export * from '../src';
```
@bvaughn
Copy link
Owner

bvaughn commented Oct 1, 2018

Okay. As much as I dislike having to do this, I think it's the only viable option at least for the short term and I appreciate how annoying it must be to not have Flow coverage for this lib, so let's do it. Thanks for your patience.

@bvaughn bvaughn merged commit 91b6909 into bvaughn:master Oct 1, 2018
@TrySound
Copy link
Contributor Author

TrySound commented Oct 1, 2018

I'm also thinking about inferring itemData inside item. Currently we have any there.

@TrySound TrySound deleted the distribute-types branch October 1, 2018 17:35
@bvaughn
Copy link
Owner

bvaughn commented Oct 1, 2018

I dig it.

@bvaughn
Copy link
Owner

bvaughn commented Oct 1, 2018

Flow types have been included with the 1.2.0 release

@KevinGrandon
Copy link

🎉 I haven't used this library yet, but just wanted to say thanks for landing flow typings! We've been following a similar approach for all of our fusion packages and it's worked out quite well for us.

@bvaughn
Copy link
Owner

bvaughn commented Oct 1, 2018

That's nice to hear @KevinGrandon. Thanks!

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

Successfully merging this pull request may close these issues.

None yet

3 participants