Strip flow-runtime imports in production mode

[doctyper]

This is a:

• Bug Report
• Feature Request
• Question
• Other

Which concerns:

• flow-runtime
• babel-plugin-flow-runtime
• flow-runtime-validatosr
• flow-runtime-mobx
• flow-config-parser
• The documentation website

---

What is the current behaviour?

It's currently not possible to strip developer-added flow-runtime imports in production. This limitation makes it difficult to manually add flow-runtime checks for teams that only type check in development mode, as it makes the flow-runtime package a requirement in production.

---

What is the expected behaviour?

If possible there should be a user-configurable way to remove user-specified flow-runtime imports / code in environments (prod / stage / test / etc).

---

Which package versions are you using?

0.2.1

[phpnode]

Hmm, why do you import the module if you're not using it? Was it to work around the issue in #26 ? If so that problem should be fixed.

[doctyper]

Well that's when I first noticed the limitation. I don't need that particular work-around anymore, but I think the use-case is valid.

For example, if I want to use flow-runtime-mobx as a development safeguard:

import t from 'flow-runtime';
import * as mobx from 'mobx';
import flowRuntimeMobx from 'flow-runtime-mobx';

flowRuntimeMobx(t, mobx); // only need to do this once.

This works of course, but I've now made flow-runtime and flow-runtime-mobx requirements for production.

I can flag these under NODE_ENV=development, but that would mean switching to requires and adding a lot of extra noise to make this dev-only.

[phpnode]

Ah ok, this is a bit tricky. Let's say we introduced a new option called stripImports and it works as you describe - removing all calls to flow-runtime and flow-runtime-mobx in production. How should this behave if the developer specifies a pragma in a file, like /* @flow-runtime assert */ ? In that scenario they are explicitly opting in to use flow-runtime in that file, but that file might not be the same file that imports and runs flow-runtime-mobx, and so it would work in development but not in production.

I'm not sure there's a way to do this automatically without turning it into a footgun. What I typically do is have a file called something like devMode.js which I selectively require in the entry point with:

if (process.env.NODE_ENV === 'development') {
  require('./devMode'); // eslint-disable-line
}

In devMode.js I can now set up flow-runtime-mobx, logging etc and dead code elimination will remove the require in production. Maybe we should just document this pattern instead?

[doctyper]

I was thinking more like babel-plugin-typecheck's disable property, where you could explicitly opt in/out of environments. {production: false} would both disable assertion and remove imports. In my opinion, that would be declarative enough that the developer can expect their checks to be disabled in specified environments. I agree that granular option checks would get muddy, but having an all-encompassing option like environment filtering would be a way to solve that.

What I typically do is have a file called something like devMode.js which I selectively require in the entry point with:

This is a suitable workaround for small-scale projects, but is not great at scale. I work with 15 web engineers across several different projects in a monorepo. In order to evangelize and increase Flow adoption, I'm looking to make it as painless as possible to adopt it. Having to add workarounds to use this feature would be a hard sell.

The flow-runtime-mobx example is just a small sample. I'd love to enable our devs to utilize your plugin's full API, but unless I can provide them an un-intrusive way to disable production use, we're limited to propType checks at runtime.

[phpnode]

If I'm understanding you correctly, you'd ideally like to be able to do something like this:

import t from 'flow-runtime';

function demo (a) {
  t.string().assert(a); // direct usage, not a transformation of a flow annotation
  return a;
}

and have the calls to t and the import removed in production, leaving you with this:

function demo (a) {
  return a;
}

Is that correct? It's pretty trivial in the basic case but gets impossibly complicated/impossible as soon as you pass the result of a call to t.something() anywhere, consider:

import t from 'flow-runtime';

function demo (a) {
  return t.typeOf(a);
}

[doctyper]

Hm, yeah I didn't think of that.

[phpnode]

Another point is that you could just do this directly, without putting it in a separate file:

import t from 'flow-runtime';
import * as mobx from 'mobx';
import flowRuntimeMobx from 'flow-runtime-mobx';

if (process.env.NODE_ENV === 'development') {
  flowRuntimeMobx(t, mobx); // only need to do this once.
}

Again, DCE and tree shaking should remove all of that.

[christophehurpeau]

You could also use something like babel-plugin-discard-module-references with babel-plugin-minify-replace to remove dev-only imports

[phpnode]

Yeah, I think it's out of scope for babel-plugin-flow-runtime, there are too many caveats with doing it automatically and lots of potential work arounds.