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

When exposing flow definitions with forceFlow maybe source files needs to be copied with .flow prefix #50

Open
DianaSuvorova opened this issue Sep 16, 2020 · 4 comments

Comments

@DianaSuvorova
Copy link

So the actual raw javascript files are not made public/accessible in the distributable unintentionally.

cc @lhorie

@rtsao
Copy link
Owner

rtsao commented Sep 17, 2020

So you are suggesting appending a .flow file extension suffix to each file in src before publishing?

The tricky thing is I think then paths/imports would have to be rewritten. I'm curious to learn more about why publishing the original source be an issue, can you elaborate?

@DianaSuvorova
Copy link
Author

DianaSuvorova commented Sep 17, 2020

@rtsao , I see what you mean. I guess your strategy is a bit different here. Since you create a brand new index.js.flow file that references the src folder. I am actually not exactly sure why did you got that route...

What I have seen people do is they provide untouched raw js.flow files alongside with compiled ones.

So currently the structure cup generates looks like this for example:

- dist-node-cjs
-- client.js [transpiled]
-- index.js [transpiled]
-- index.js.flow [export * from "../src/index.js";]
- src
-- index.js [raw]
-- client.js [raw]

What I have seen libs do is the following:

- dist-node-cjs
-- client.js [transpiled]
-- client.js.flow [raw client.js]
-- index.js [transpiled]
-- index.js.flow [raw index.js]

Then you shouldn't need to do anything about paths, and you avoid exposing your raw code to the consumers (ie it is not accessible via js require\import).

There is a util to do this. Baseweb has a custom script

Here is github discussion about the best strategies to expose flow types for a lib

@rtsao
Copy link
Owner

rtsao commented Sep 17, 2020

I see, thanks. I think the original reasoning for the current design is twofold:

  • Less copying (only a single extra file needs to be created)
  • Less ambiguity if there is explicit type definitions, (e.g. there is bothsrc/foo.js and src/foo.js.flow)

I felt like the current approach is the simplest implementation. I think your suggestion would also work fine though. If you wanted to put up a PR, I'd be happy to review!

Would you help me understand the motivation behind hiding the original source? If consumers are importing that directly, isn't that their prerogative?

I suppose the issue is it potentially implies (falsely) that the src/ is part of the API contract of the package? And you want to restrict the surface area to the compiled source only?

@DianaSuvorova
Copy link
Author

suppose the issue is it potentially implies (falsely) that the src/ is part of the API contract of the package? And you want to restrict the surface area to the compiled source only?

Yes, exactly. I can extend on this reasoning, but this is exactly the point.

Ok, I will craft a PR. Thank you 😊

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

No branches or pull requests

2 participants