this is literally copied from here any suggestions what's the best way to dedupe this?

You can create a path.splitExportDeclaration() function (or a @babel/helper-...)

was thinking about both, weren't sure which one to pick :D I'd probably go with latter, thanks

hating this IfStatement here, kinda a hack to insert a correct return for mentioned double IIFEs case - need to figure out how to flatten those IIFEs (in case of ClassExpresion with static properties) and keep all surrounding references correct

Is it possible to remove this unused _class = assignment?

it probably is, added _class is caused by ClassExpression being replaced by multiple statements. The path#replaceWithMultiple creates an IIFE when the conversion is made so the single expression can be maintained in place.

This is certainly something I'd like to revisit, but for another month (till Jan 16th) I have very little time and I couldn't manage to do this properly at the moment because of the complexity and probably my insufficient experience with AST manipulation.

Ideally I would like to "inline" the ClassExpression here if possible to skip second IIFE creation, but I'm not sure how safe it is because scope references have to be synced and stuff.

This case (and other similar) are caused by simple insertion here. To remove this unused variable the logic here would have to be expanded. I might revisit this in a followup PR, but would like to keep this one as simple as possible.

Isn't this iife pure only if computed properties are pure?

Yes, you are right. They can also be impure in other cases, such as impure property initializers, or even super class being an impure expression.

I've wanted to keep marking those class IIFEs as pure, because it is what transform-classes do at the moment in all cases (and classes without properties can be impure too). That's quite my fault, adding those annotations was one of my 1st PRs to babel.

I'd like to revisit (as another PR) annotating classes as pure as a whole and not just in case of proposal-class-properties plugin. While checking if class is pure should be quite easy, I'm not sure what we should do about subclasses - we won't be able to tell if a super class is pure or not.

Can we split the class and the export statement only if it is actually needed? (Only if it doesn't add too much complexity to the plugin)

It was easier to split those for now in all cases, need to check how much complexity it would add to keep them together (and the same should be done in transform-classes as it is splitting those every time)

export default function() {}

is transformed into

function _default() {}
export default _default;

The first export statement is hoisted, but is it also the second?

could u elaborate? I've thought about hoisting briefly in this context, but came to conclusion that no extra work needs to be done for export statements, they are evaluated in place, right? and as function declaration will get hoisted by javascript this identifier will be available by the time it gets exported

After playing a bit with it, I understood that the way you implemented it is correct 👍

Nit: can you use a default export? It seems that most helpers do that.

Can you use reexport.bindings.forEach({ name } => { and avoid an unnecessary loop? Also on lines 186 and 200

Here you can avoid looping twice:

for (const { name } of data.bindings) {

Aren't identifiers normal expressions? e.g., what is the difference between

var a = 2;
export default a;

and

export default 2;

?

There is no difference when it comes to the functionality.

The containing function here (getLocalExportMetadata) gathers export names for local bindings so every future assignment to those local bindings can be transformed into a live reference.

You can note that this function here expects only a FunctionDeclaration, ClassDeclarations and now Identifiers in position of default export. Everything else in this position gets splitted before by nameAnonymousExports into

var *local* = expression
export { *local* as default }

I omit this splitting for local bindings here to produce a nicer output, if I'd keep splitting them we would end up with things like:

var _default = function () { /* some class */ }
var _default2 = _default
module.exports = _default2

I'd prefer this to throw if assumePure is specified and it is neither a boolean nor null/undefined.

Gonna go with throwing on nulls too, it doesn't give any extra meaning for the option, and one can just skip providing it (thus making it undefined). If you feel we should allow null here and treat is as undefined, please raise your concern.

You can just use path.insertAfter(nodes), thanks to #7040

You should always generate a new identifier, otherwise it won't work in cases like https://babeljs.io/repl/build/6407/#?babili=false&browsers=&build=&builtIns=false&code_lz=DYUwLgBAghC8ECYDcAoFAzAdgCgMbAEMBnI6CAbxQgiLALAEtcIAHAJwHsW5FUBfAJRIgA&debug=false&circleciRepo=&evaluate=false&lineWrap=false&presets=stage-0&prettier=false&targets=&version=7.0.0-beta.36%2Bpr.6963 (A gets declared twice).

You can do something like

ref = path.scope.generateUidIdentifier(path.node.id ? path.node.id.name : "class");

Good catch. Need to dig into this after work, as I have a feeling that it will break inlining ClassExpressions into a class IIFE here, so I will have to check this and probably fix.

Made a quick test and as suspected - it breaks some cases. Maybe you would have a possible solution/approach in mind to fix this.

The problem is that this:

() => class Bar {
  method(){}
  static fn = () => 'static';
};

gets transformed into with class-properties transform

() => {
  var _Bar;
  return _Bar =
  /*#__PURE__*/
  (() => {
    let _Bar = class Bar {
      method() {}
    };
    _Bar.fn = () => 'static';
    return _Bar;
  })();
}

which gets later transformed with transform-classes into:

() => {
  var _Bar;
  return _Bar =
  /*#__PURE__*/
  (() => {
    function Bar() {}
    var _proto = Bar.prototype;
    _proto.method = function method() {};
    _Bar.fn = () => 'static';
    return _Bar;
  })();
};

The problem lies in the fact that the mentioned inlining doesn't know that the local class ID is other than that class ID itself.

What comes to my mind is to:

• retrieve this ID - we know for sure in this place that the parentStatement (the one that is going to be replaced) is a VariableDeclaration containing the correct ID
• rename references to the old ID (constructor's ID) with the retrieved ID (Im not sure how to do this right now, as I'd like to update identifiers in array of nodes and not a path)
• actually do not remove parentStatement, but just replace it's init with a constructor (from the nodes that we want to insert here), to keep a local ID and a class proper name

Probably the right fix is avoiding adding var [name] for class expressions.

e.g.
https://babeljs.io/repl/build/6407/#?babili=false&browsers=&build=&builtIns=false&code_lz=BQYwNghgzlAECCsDeAoWsoBcKYJYlgDMB7Y2AXlgCYBuFAXwEog&debug=false&circleciRepo=&evaluate=false&lineWrap=false&presets=stage-0&prettier=false&targets=&version=7.0.0-beta.36%2Bpr.6963

Input:

(class A {
  static foo = 2;
})

Expected output

/*#__PURE__*/
(() => {
  let A = class A {};
  Object.defineProperty(A, "foo", {
    configurable: true,
    enumerable: true,
    writable: true,
    value: 2
  });
  return A;
})();

// Which then becomes

/*#__PURE__*/
(function () {
  var A = function A() {
    _classCallCheck(this, A);
  };

  Object.defineProperty(A, "foo", {
    configurable: true,
    enumerable: true,
    writable: true,
    value: 2
  });
  return A;
})();

Actual output

var A;

A =
/*#__PURE__*/
(() => {
  let A = class A {};
  Object.defineProperty(A, "foo", {
    configurable: true,
    enumerable: true,
    writable: true,
    value: 2
  });
  return A;
})();

// Which then becomes

var A;

A =
/*#__PURE__*/
function () {
  var A = function A() {
    _classCallCheck(this, A);
  };

  Object.defineProperty(A, "foo", {
    configurable: true,
    enumerable: true,
    writable: true,
    value: 2
  });
  return A;
}();

Q: What is outer ref needed for? It doesn't conflict now so it works, but I still don't understand what it is for.

Im pretty confident it's not needed in 100% cases, it was here before the PR - didnt want to optimize the output more than needed. Gonna revisit this in followup PR.

Is this the only part that you do not understand? Or is something else needing a clarification too?

I think it was needed because static properties where defined after the iife: https://babeljs.io/repl/build/master#?babili=false&browsers=&build=&builtIns=false&code_lz=BQYwNghgzlAECCsDeAoWsoBcKYJYlgDMB7Y2AXlgCYBuNWAWwFNMALYgE2AEpkBfFH240gA&debug=false&circleciRepo=&evaluate=false&fileSize=false&lineWrap=false&presets=es2015%2Cstage-0%2Ctypescript&prettier=false&targets=&version=7.0.0-beta.36 (_class is used in Object.defineProperty)

Is this the only part that you do not understand? Or is something else needing a clarification too?

Thank you, the other parts are clear enough 😊

Even if it doesn't have an ID, cyclic dependencies can still rely on it being hoisted. It's never safe to do export default foo; in this case.

could you back it up with a sample code? not sure if I understand how the problem can be created

I generally haven't been super comfortable with things that rely on being singletons like this because it relies on them being the same version and npm flattening things. Usually we've relied on flattening for better performance, but making flattening affect the actual transform output worries me a lot.

good point, but without merging both class transforms (which I think @hzoo proposed numerous times) i have no idea how to achieve the desired goal of "pulling" stuff into a single IIFE here

@loganfsmyth do you have an idea how I could tackle this one differently?

The only thing that comes to my mind is to wrap a class with IIFE in class-properties transform and hardcode some detection logic in transform-classes to check if it should create IIFE or if it should just inline output nodes in place (when it would know that it's already in the IIFE).

It would create indirect dependency on what class-properties does, but even if the detection logic fails it would just create an extra IIFE (so the output still would be valid, just not "optimized").