Skip to content
This repository has been archived by the owner on Aug 4, 2021. It is now read-only.

Better handling of helpers #19

Merged
merged 5 commits into from
Dec 29, 2015
Merged

Better handling of helpers #19

merged 5 commits into from
Dec 29, 2015

Conversation

Rich-Harris
Copy link
Contributor

This addresses #17 by allowing runtime helpers if applicable via a runtimeHelpers: true option. It's also a bit more permissive about helpers generally – it will only complain about the lack of external-helpers-2 if helpers are in fact duplicated, and it will warn instead of throwing an error.

It also points to some better documentation explaining why these are needed.

@Victorystick @eventualbuddha does this seem like the right direction?

@Rich-Harris
Copy link
Contributor Author

Oh and it changes babelHelpers.longWindedHelperName to babelHelpers_longWindedHelperName. Bit more minifiable

@@ -46,7 +44,7 @@ describe( 'rollup-plugin-babel', function () {
});
});

it( 'adds helpers', function () {
it.only( 'adds helpers', function () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove debug code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch! fixed

@Victorystick
Copy link
Contributor

Looking good! Lots of improvements going down lately! 😉

Victorystick added a commit that referenced this pull request Dec 29, 2015
Better handling of helpers
@Victorystick Victorystick merged commit 2fc82d6 into master Dec 29, 2015
@eventualbuddha
Copy link
Contributor

👍 Haven't tried it, but seems good.

@Rich-Harris Rich-Harris deleted the gh-17 branch December 30, 2015 02:25
@ericf
Copy link
Contributor

ericf commented Jan 7, 2016

Oh and it changes babelHelpers.longWindedHelperName to babelHelpers_longWindedHelperName. Bit more minifiable

@Rich-Harris something is going wrong with this change I'm seeing the following for the createClass helper which ends with an invalid }();:

  function babelHelpers_createClass () {
    function defineProperties(target, props) {
      for (var i = 0; i < props.length; i++) {
        var descriptor = props[i];
        descriptor.enumerable = descriptor.enumerable || false;
        descriptor.configurable = true;
        if ("value" in descriptor) descriptor.writable = true;
        Object.defineProperty(target, descriptor.key, descriptor);
      }
    }

    return function (Constructor, protoProps, staticProps) {
      if (protoProps) defineProperties(Constructor.prototype, protoProps);
      if (staticProps) defineProperties(Constructor, staticProps);
      return Constructor;
    };
  }();

This change is also making the function declarations end with a ;:

  function babelHelpers_classCallCheck (instance, Constructor) {
    if (!(instance instanceof Constructor)) {
      throw new TypeError("Cannot call a class as a function");
    }
  };

I think you should rename them to babelHelpers_*, but keep them as function expressions.

ericf added a commit to ericf/rollup-plugin-babel that referenced this pull request Jan 7, 2016
The renaming from `babelHelpers.foo =` to a function declaration
`function babelHelpers_foo()` was incorrect as the helpers are
defined as expressions.

This puts back the helper functions being defined as function
expressions but keeps the better-minifiable `babelHelpers_` bindings.

See: rollup#19 (comment)
@ericf
Copy link
Contributor

ericf commented Jan 7, 2016

PR to fix the above: #31

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants