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

Support flow:main in manifest #2675

Closed
steelbrain opened this issue Oct 24, 2016 · 11 comments
Closed

Support flow:main in manifest #2675

steelbrain opened this issue Oct 24, 2016 · 11 comments
Labels
declarations Issues with library definitions or .js.flow features feature request

Comments

@steelbrain
Copy link

(I searched for an existing issue but couldn't find one so sorry if this is a duplicate)

The problem

The problem is that we want flow to understand the types of flow-typed modules correctly when requiring them.

The workaround

We currently have to keep .flow files along with .js files after they are babel compiled and that adds an extra build step and a ton of headache.

The solution

If Flow has support for flow:main (like jsnext:main for rollup), we would not have to add anything new to our build step. We can just leave the src dir in our npm package and set flow:main to src/.

This is a follow up to #1459

@steelbrain
Copy link
Author

steelbrain commented Aug 5, 2017

@STRML About your comment on the PR, AgentMe/flow-copy-source doesn't integrate well with babel so I've forked the babel cli and created a dropin replacement babel-cli-flow, it works well even with the watcher and updates $.flow copies instantly

@TrySound
Copy link
Contributor

You might don't need to look for module field. Use this way

{
  "main": "./lib/index.js",
  "module": "./esm/index.js",
  "files": ["lib", "esm", "src"],
  "scripts": {
    "build:flow": "echo \"// @flow\\n\\nexport * from '../src';\" > lib/index.js.flow"
  }
}

@wtgtybhertgeghgtwtg
Copy link

wtgtybhertgeghgtwtg commented Mar 10, 2018

I think that's what he was addressing under "The workaround". It's certainly an option some libraries are taking, but it feels hacky and potentially increases bundle package size.

@TrySound
Copy link
Contributor

@wtgtybhertgeghgtwtg How this increases bundle size? lib/index.js.flow file is used only by flow. Bundlers do not care about it. Also with this you don't need to duplicate types anywhere. Sust place one .js.flow reference near entry point.

@TrySound
Copy link
Contributor

Also there is #5729 with similar feature, so this can be closed

/cc @vkurchatkin

@wtgtybhertgeghgtwtg
Copy link

Sorry, I meant package size.
I don't think #5729 solves this issue. Ideally, Flow types should work without an extra build step and without the consumer having to alter their configuration.

@TrySound
Copy link
Contributor

Package size? With a few bytes? Who cares? My way is pretty good for this without any new fields in package.json.

@wtgtybhertgeghgtwtg
Copy link

Yeah, I completely misinterpreted what the script did. I still feel that this would be a better solution, but I'm going to start using that script.

@TrySound
Copy link
Contributor

The problem is that own entry point does not solve necessary cases. For example:

import map from 'lodash/map'

requires map.js.flow file. Such cases are not solvable with simple configuration. But with .js.flow and main field we have universal way to define types. So I consider it as non-hacky and even more productive than any config and additional fields.

@steelbrain
Copy link
Author

After trying out all of the popular third party fixes for this issue, I would like to state that all of them have their quirks. It feels unnecessary to have to create .js.flow for each file you transpile in your project.

There are some use cases where we want interop between TS/Flow and maybe even others, but that's a special case and I think .js.flow handles it quite well since you would be writing it by hand or using a tool to translate flow types to typescript or vice versa.

But the original issue was about being able to require your dependencies, that all use flow, and having first party support in package.json to point flow to the source files, that babel itself is compiling from.

It would solve cases like lerna/lerna#891 #5975 among others. Using third party tools to map source files to .js.flow files when we do have the ability to use the source files directly (see #1459) should not be what we try to accomplish. flow:main or a similar field in manifest to tell flow where to look for sources would ease the pain of many :)

@steelbrain
Copy link
Author

I've opened a PR to fix this in #6504

@goodmind goodmind added the declarations Issues with library definitions or .js.flow features label Jun 22, 2019
@SamChou19815 SamChou19815 closed this as not planned Won't fix, can't repro, duplicate, stale Mar 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
declarations Issues with library definitions or .js.flow features feature request
Projects
None yet
Development

No branches or pull requests

6 participants