Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Babel helper export is wrongly hoisted #2414

Closed
TrySound opened this issue Aug 18, 2018 · 15 comments · Fixed by #2502
Closed

Babel helper export is wrongly hoisted #2414

TrySound opened this issue Aug 18, 2018 · 15 comments · Fixed by #2502

Comments

@TrySound
Copy link
Member

Is rollup able to code split inlined babel helpers?

I got an issue with this. This helper is exported at the top of the chunk with exports({ e: _extends })

var _extends = Object.assign || function (target) { for (var i = 1; i < arguments.length; i++) { var source = arguments[i]; for (var key in source) { if (http://Object.prototype.hasOwnProperty.call (source, key)) { target[key] = source[key]; } } } return target; };

I guess it's inlined helper. Babel distributes babe-runtime with not minified helpers.

Moving exports({}) to the bottom of the chunk should solve the problem.

And I don't know how to reproduce this in small sandbox.

@lukastaegert
Copy link
Member

It would be nice to have a little more information.

  • Which output format does this apply to?
  • In which situations does this happen?
  • Do you have a repo or test case that exposes the problem? Otherwise there is no way for us to determine if a fix is working and I hate to do guess-work here.

@TrySound
Copy link
Member Author

It's systemjs. I don't know why, this happens when I add babel-plugin-transform-runtime.

As I said I don't know how to reproduce this with small sandbox. It fails only in my big app with special case. So I'm asking is rollup able to split consumed files in the middle?

My problem is splitted inlined helper which cannot hoist and considered by rollup as hoisted.

@TrySound
Copy link
Member Author

@lukastaegert @guybedford I found how to reproduce bug. Not sure it's exactly same bug though.
https://github.com/TrySound/rollup-bug-1

@TrySound
Copy link
Member Author

TrySound commented Oct 8, 2018

@lukastaegert This just broke my app completely and unconditionally today. I can only remove some pages to fix it. Really annoying problem.

@TrySound
Copy link
Member Author

TrySound commented Oct 8, 2018

@lukastaegert I removed all plugins except commonjs. Hope this will simplify your investigation.

@guybedford
Copy link
Contributor

@TrySound it may help if you can share the build output, and do as much as you can to cut down the size of the build by emptying all modules that aren't affecting the issue.

@TrySound
Copy link
Member Author

TrySound commented Oct 8, 2018

@guybedford Reduced a lot. Take a look please if you have a time.

@TrySound
Copy link
Member Author

TrySound commented Oct 8, 2018

(function () {
  'use strict';

  function _extends$1() {
    _extends$1 = Object.assign || function (target) {
      for (var i = 1; i < arguments.length; i++) {
        var source = arguments[i];

        for (var key in source) {
          if (Object.prototype.hasOwnProperty.call(source, key)) {
            target[key] = source[key];
          }
        }
      }

      return target;
    };

    return _extends$1.apply(this, arguments);
  }

  var whitelist = ['en', 'fr'];
  var versions = whitelist.reduce(function (acc, lng) {
    var _extends2;

    return _extends({}, acc, (_extends2 = {}, _extends2[lng] = null, _extends2));
  }, {});

}());

@lukastaegert
Copy link
Member

Ok, so this looks like _extends is referenced as _extends$1 as if it has been deconflicted against itself.

@lukastaegert
Copy link
Member

It seems the issue is related to the fact that for the reassignment, a LocalVariable is used/introduced that is disconnected from the ExportDefaultvariable generated by the export.

@lukastaegert
Copy link
Member

Will keep investigating tomorrow.

@TrySound
Copy link
Member Author

TrySound commented Oct 9, 2018

Thanks. At least I found a way to fix the issue. I just overrode extends helpers in transform hook.

@lukastaegert
Copy link
Member

Potential fix at #2502. Could you check if this also fixes the original issue?

@TrySound
Copy link
Member Author

Just tried. Works perfectly. Thank you

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

Successfully merging a pull request may close this issue.

3 participants