Fix default export with arrows and function naming

[danharper]

Q	A
Bug fix?	yes
Breaking change?	no
New feature?	no
Deprecations?	no
Spec compliancy?	no
Tests added/pass?	yes
Fixed tickets	#4377, #4182, #4430
License	MIT
Doc PR	-

Default arrow function exports, with transform-es2015-arrow-functions and transform-es2015-function-name (or, just the es2015 preset) results in loss of the default export.

• function-name renames existing clashing identifiers when adding a function name.
• When renaming, it attempts to separate export declarations.
• In doing so, as the transpiled arrow function has no name, the default export node gets changed to a named export which has no name.

In the tests, I've only added ones for the CJS output as that's what I was originally trying to solve. This actually affects all module formats. I can follow up with duplicate tests for each module system if you'd like.

---

Minimal reproduction of the issue:

{
  "plugins": [
    "transform-es2015-function-name",
    "transform-es2015-shorthand-properties",
    "transform-es2015-arrow-functions"
  ]
}

export default (a) => {
  return { a() { return a } };
}

(function (_a) {
  return {
    a: function a() {
      return _a;
    }
  };
};

export {}

[codecov-io]

Current coverage is 88.33% (diff: 100%)

Merging #4524 into master will increase coverage by <.01%

@@             master      #4524   diff @@
==========================================
  Files           194        194          
  Lines         13616      13617     +1   
  Methods        1427       1427          
  Messages          0          0          
  Branches       3147       3148     +1   
==========================================
+ Hits          12027      12029     +2   
+ Misses         1589       1588     -1   
  Partials          0          0          

Powered by Codecov. Last update e64d86c...41a3d95

[danez]

Thank you. If you can easily reproduce this bug with amd and umd, then it would be nice to have a simple test for each of the module-systems.
I had a look at the code, but haven't had a look before at renamer, so going to debug a little bit, so I understand what it is doing.

[danharper]

👍 will do, just wanted to check this is the correct approach first.

An alternative approach is to transpile arrow functions whose parent is an ExportDefaultDeclaration into a FunctionDeclaration instead of a FunctionExpression. At which point, the renamer will ensure it has a name of _default. Then we don't need to check for specifiers.length, as the arrow will have a name.

function _default(_a) {
  return {
    a: function a() {
      return _a;
    }
  };
}
exports.default = _default;

[danharper]

However, looking into it more, the code giving default exports a name was introduced here: #3134, which seems to be the same issue this is resolving

it was inserting a standalone export {}

If I remove that code, everything continues to work with my changes, just the associated test case changes to not be a separate statement:

  "use strict";

  Object.defineProperty(exports, "__esModule", {
    value: true
  });
+ exports.default = function () {
- function _default() {
    return function () {
      function Select() {
        babelHelpers.classCallCheck(this, Select);
      }

      babelHelpers.createClass(Select, [{
        key: "query",
        value: function query(_query) {}
      }]);
      return Select;
    }();
  }
- 
- exports.default = _default;

(I'll check tonight how that works with class declarations, too.)

[hzoo]

The regression was #4228 with example code: export const state = (state) => state.a

[danharper]

Yeah, that was a regression from a previous attempted fix for exported arrows, which got reverted.

[hzoo]

This is great thanks!!