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

Ability to copy all files and not only JS files when stripping types using flow-remove-types #9148

Open
bacloud22 opened this issue May 14, 2024 · 5 comments

Comments

@bacloud22
Copy link

Proposal

I think it is useful to have all files copied by default when using flow-remove-types. I for instance use .proto files inside code, you can imagine other useful extensions that generally go hand in hand with JS (like JSON, HTML..)

Use case

My build step is like "build": "flow-remove-types src/ -d dist/",, source folder contains files other than .js so the ./dist is not self contained anymore.

@SamChou19815
Copy link
Contributor

Note that we probably won't do it ourselves, since we don't really recommend using it. The implementation of flow-remove-types is quite hacky, and you will be better off using babel directly. We do accept community contribution.

@bacloud22
Copy link
Author

I would like to work on this. Please assign @SamChou19815 🙏

@bacloud22
Copy link
Author

bacloud22 commented May 15, 2024

OK understood about recommendation. But won't babel approach use the same whatever algorithm to remove types ?
I would like not to add babel at all, since I've never used it, and I don't need it.

Do you mean using flow-remove-types is not recommended at all ? For me it seems to build JS without types just fine. I've added a hacky way to preserve all files:
"build": "rm -rf dist/ && cp -a src/ dist/ && flow-remove-types src/ -d dist/"
first it copies all files and folders (including JS, not the best) then it ovewrites JS with the ones from flow-remove-types

@SamChou19815
Copy link
Contributor

But won't babel approach use the same whatever algorithm to remove types

No, babel will remove the types from the AST, and then print the result API back to code. That approach is much less fragile then the string manipulation approach based on ranges that's currently in flow-remove-types.

The only thing preventing us from deleting it from the repo is that we have not find an easy way to use the babel infra so that flow-remove-types powered flow-node, which can directly run flow code with the correct line number in stacktraces, still works.

The energy would be better spent on making the babel-based approach work by connecting it to the source map. That being said, we will still accept bug fixes and improvements to flow-remove-types before we work out the babel stuff.

@bacloud22
Copy link
Author

Hi @SamChou19815
Understood! And thank you for explanations.

I still can work on this like you said, but it might take time for me to put my hands on code..

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

No branches or pull requests

2 participants