Named export arrow function name inference

[tomchentw]

Description

When a React component is defined as a named export arrow function, the name of it cannot be inferred correctly in jest coverage mode. (To be precise, it's node that cannot infer it) It generates inconsistent result compared with the stored snapshots.

What's a named export arrow function?

export const MyFancyComponent = props => <div {...props} />

Digging in

It turns out the above MyFancyComponent will be transpiled (proper babel-preset-env) into this:

var MyFancyComponent = exports.MyFancyComponent = (cov_15qdocbbym.s[0]++, props => {
  cov_15qdocbbym.f[0]++;
  cov_15qdocbbym.s[1]++;
  return <div {...props />;
});

where the instrument code is inserted along with the arrow function, generating a SequenceExpression. However, node (I'm using 8.2.1) cannot infer the name of the arrow function inside a sequence expression.

> const a = (1, () => 2);
undefined
> a.name
''
> const b = () => 3;
undefined
> b.name
'b'

A possible solution

Modify the insertCounter function in istanbul-lib-instrument/src/visitor.js as follows:

-            if (parent && (T.isProgram(parent.parentPath) || T.isBlockStatement(parent.parentPath))) {
+            if (parent && T.isExportNamedDeclaration(parent.parentPath)) {
+                parent.parentPath.insertBefore(T.expressionStatement(increment));
+            } else if (parent && (T.isProgram(parent.parentPath) || T.isBlockStatement(parent.parentPath))) {

Thus, the generated code would be something like:

cov_15qdocbbym.s[0]++;
var foo = exports.foo = props => {
  cov_15qdocbbym.f[0]++;
  cov_15qdocbbym.s[1]++;
  return 'bar';
};

Thus, node can infer foo now :)

Questions

• What's the current policies for ES modules of istanbul projects?
• Is it a good practice to make such changes for ES modules compatibility?
• Would you accept a PR?

Repos

I put my changes of istanbul-lib-instrument into my forked repo:

• tomchentw/istanbuljs

I also published it as a scoped npm module under @tomchentw/istanbul-lib-instrument, hence I can forked babel-plugin-istanbul to add a failing case and fix it:

• tomchentw/babel-plugin-istanbul

Related issues

#63
……and maybe more.

[bcoe]

@tomchentw within reason we've been trying to support ES modules and other bleeding edge ES features provided by babel -- I'd definitely accept a pull request, with the caveat that we should be fairly careful, since we've accidentally created invalid syntax in the past trying to address the issue of function name preservation.

A good way to interact with me directly is in the slack I've created for the various devtools I help manage:

http://devtoolscommunity.herokuapp.com/

[tomchentw]

I can confirm this is fixed by #126.

For jest users, please ran yarn upgrade and run jest with --no-cache option.

[jdolle]

@tomchentw Upgrading and running jest with --no-cache does "fix" the issue, but I don't see why --no-cache should be necessary and I also don't think the issue is actually fixed until I can run jest without the --no-cache option. There is a drastic difference in how long my tests take to run on CI when using cache and not -- to the point of --no-cache not being viable.

I'd like to request the issue be reopened.

[ljharb]

That's because of issues with jest caching; it should be a separate issue.

[thymikee]

@jdolle jestjs/jest#1740 (comment)