Proposal: use ref(() => Map) instead of ref('Map')
This is a:
- [ ] Bug Report
- [ ] Feature Request
- [x] Question
- [ ] Other
Which concerns:
- [x] flow-runtime
- [x] babel-plugin-flow-runtime
- [ ] flow-runtime-validators
- [ ] flow-runtime-mobx
- [ ] flow-config-parser
- [ ] The documentation website
Usingref with the name of a type as a string is fundamentally problematic
Because the babel module transform mangles the names of imports. It has to because it can't know at compile time if the value of an export will change, because of circular imports, etc.
So any code that babel-plugin-flow-runtime converts to ref('Map') will break if Map comes from import Map from '...' because that import will get transpiled to
var _map = require('...')
var _map2 = _iteropRequireDefault(_map)
(see #177)
I think the best way to get around this would be to use a function instead of a string: ref(() => Map). That way it can still be evaluated at runtime, but babel's module transform will convert that to ref(() => _map2.default), so it won't break.
@phpnode thoughts?
We use a string ref (in this case) because at this stage we don't know whether Map is a concrete value or a type. I guess we could generate something like t.ref(() => typeof Map === 'undefined' ? 'Map' : Map) though
Oh, I see. Well babel-plugin-flow-runtime is converting types to concrete values anyway; only types imported from external code compiled without babel-plugin-flow-runtime would not be a concrete value. So wouldn't t.ref(() => typeof Map === 'undefined' ? null : Map) work?'
And if it does work, really, we could just use () => Map and have flow-runtime catch the TypeError if Map is undefined.
babel-plugin-flow-runtimeis converting types to concrete values anyway;
This is true most of the time, but not for global type definitions, for example if something is an Iterable<T> we don't import that directly and can't reference it by value, instead we use the type's name to look it up in our registry. The registry is built by running flow-runtime-cli. So we can't assume that every type will be a value we can directly reference, and if we can't reference the value we need a good error message telling the user which type is missing, so we do need the type name as a string.
Ah, that makes sense.
I see that registerBuiltins refers to Map directly (https://github.com/codemix/flow-runtime/blob/master/packages/flow-runtime/src/registerBuiltins.js#L67) so actually, if I processed flow-runtime with babel-plugin-transform-runtime in my code I bet it would work as-is.
If you don't mind my asking though, why not just import a pre-built runtime type directly instead of using t.ref('Iterable')? For instance:
Input
import {reify} from 'flow-runtime'
import type {Type} from 'flow-runtime'
const foo = (reify: Type<Iterable>)
const bar = (reify: Type<Map>)
Output
import { reify } from 'flow-runtime';
import { Type as _Type } from 'flow-runtime';
import { Iterable, Map as _Map } from 'flow-runtime'; // or maybe flow-runtime/builtins
import t from 'flow-runtime';
const Type = t.tdz(() => _Type);
const foo = Iterable;
const bar = _Map;
Even better, for Map, Set, and other concrete builtins, flow-runtime could actually accept the instance that's on the user's scope, which would solve the issue I was running into for sure:
const bar = _Map(Map);
This will work for known builtins but not anything that's declared in a file in the flow-typed directory, e.g. if you use express it's relatively common to use express$Request without directly importing it.
Regarding your second point - that should already be the case, for instance if you declare your own Map then flow-runtime will use it:
type Map = 123;
const map:Map = 123
compiles to:
import t from "flow-runtime";
const Map = t.type("Map", t.number(123));
const map = Map.assert(123);
This doesn't work in your scenario because babel-plugin-flow-runtime is running before babel-plugin-transform-runtime and so doesn't see the new reference to corejs.Map.
The only way to solve these issues nicely is to build webpack plugins / loaders which have access to the entire codebase, not just a single file at a time like babel does. I want to work on this but finding the time to do so is hard!
So I webpack my client code, but I don't use it on my server-side code at all; I only babelify my server-side code. I'm sure many people do the same. So I don't think webpack plugins / loaders would really be a complete solution for all JS.
Does flow-runtime currently have a way of supporting express$Request? If so, how? Maybe it's common to use it but I think it's unwise and not worth supporting because it's essentially an undocumented type declaration polluting the global namespace that's subject to change without notice.
As far as I understood, flow-runtime can't currently import runtime types from any module that doesn't export concrete runtime types (which it has already converted with babel-plugin-flow-runtime)?
The thing I was talking about with const bar = _Map(Map); was not concretely declaring a Map type like type Map = 123 but passing in the Map that's been put on the global namespace by whatever polyfills are being used (if any), so that we can make sure flow-runtime is using the same class.
Here's an idea: how about running a server in the background that caches information about types in all the source files, and requesting info about other files in the babel plugin?
(I know you're busy and I'm not demanding that you to implement any of these things yourself; I'm just discussing the design because I might eventually try to implement a solution myself)
I quite like the idea but I also don't want to reinvent Flow, the best solution would be to make Flow's type information accessible at the AST level in babel, but that's quite a challenge
Yeah I know, I just don't really see how else we would do that.
Here's another thought: we could have a preprocessing program go through all the type imports, and generate a file with all those types converted to validators if they aren't already.
The Babel plugin would then have only one place to look for importing these extra validators.
Here's another thought: we could have a preprocessing program go through all the type imports, and generate a file with all those types converted to validators if they aren't already.
That's what flow-runtime-cli does, so if you run flow-runtime generate ./src > ./src/typedefs.js it will crawl all your code and generate definitions in one file that you can later import. It is definitely not perfect though