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

v4.0.0.rc.*: Configuring Babel to target esmodules incorrectly generates ES5 instead of ES6 #1892

Closed
javan opened this issue Jan 16, 2019 · 4 comments
Assignees

Comments

@javan
Copy link
Contributor

javan commented Jan 16, 2019

Using the following source app/javascript/packs/application.js file:

class Speaker {
  constructor(message) {
    this.message = message
  }

  speak() {
    console.log(this.message, this)
  }
}

const speaker = new Speaker("hello!")
speaker.speak()

With Webpacker v4.0.0.pre.3, I could configure Babel to compile ES6 by targeting { "esmodules": true }:

.babelrc
{
  "presets": [
    [
      "@babel/preset-env",
      {
        "targets": {
          "esmodules": true
        }
      }
    ]
  ],
  "plugins": [
    "@babel/plugin-transform-destructuring",
    "@babel/plugin-syntax-dynamic-import",
    [
      "@babel/plugin-proposal-object-rest-spread",
      {
        "useBuiltIns": true
      }
    ],
    [
      "@babel/plugin-transform-runtime",
      {
        "helpers": false,
        "regenerator": true
      }
    ],
    [
      "@babel/plugin-transform-regenerator",
      {
        "async": false
      }
    ],
    [
      "@babel/plugin-proposal-class-properties",
      {
        "loose": true
      }
    ]
  ]
}
public/packs/application.js
/***/ "./app/javascript/packs/application.js":
/*!*********************************************!*\
  !*** ./app/javascript/packs/application.js ***!
  \*********************************************/
/*! no static exports found */
/***/ (function(module, exports) {

class Speaker {
  constructor(message) {
    this.message = message;
  }

  speak() {
    console.log(this.message, this);
  }

}

const speaker = new Speaker("hello!");
speaker.speak();

/***/ })

With Webpacker v4.0.0.rc.2, this no longer works:

babel.config.js
module.exports = function(api) {
  var validEnv = ['development', 'test', 'production']
  var currentEnv = api.env()
  var isDevelopmentEnv = api.env('development')
  var isProductionEnv = api.env('production')
  var isTestEnv = api.env('test')

  if (!validEnv.includes(currentEnv)) {
    throw new Error(
      'Please specify a valid `NODE_ENV` or ' +
        '`BABEL_ENV` environment variables. Valid values are "development", ' +
        '"test", and "production". Instead, received: ' +
        JSON.stringify(currentEnv) +
        '.'
    )
  }

  return {
    presets: [
      isTestEnv && [
        require('@babel/preset-env').default,
        {
          targets: {
            node: 'current'
          }
        }
      ],
      (isProductionEnv || isDevelopmentEnv) && [
        require('@babel/preset-env').default,
        {
          targets: {
            esmodules: true
          }
        }
      ]
    ].filter(Boolean),
    plugins: [
      require('babel-plugin-macros'),
      require('@babel/plugin-syntax-dynamic-import').default,
      isTestEnv && require('babel-plugin-dynamic-import-node'),
      require('@babel/plugin-transform-destructuring').default,
      [
        require('@babel/plugin-proposal-class-properties').default,
        {
          loose: true
        }
      ],
      [
        require('@babel/plugin-proposal-object-rest-spread').default,
        {
          useBuiltIns: true
        }
      ],
      [
        require('@babel/plugin-transform-runtime').default,
        {
          helpers: false,
          regenerator: true
        }
      ],
      [
        require('@babel/plugin-transform-regenerator').default,
        {
          async: false
        }
      ]
    ].filter(Boolean)
  }
}
public/packs/application.js
/***/ "./app/javascript/packs/application.js":
/*!*********************************************!*\
  !*** ./app/javascript/packs/application.js ***!
  \*********************************************/
/*! no static exports found */
/***/ (function(module, exports) {

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

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); } }

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

var Speaker =
/*#__PURE__*/
function () {
  function Speaker(message) {
    _classCallCheck(this, Speaker);

    this.message = message;
  }

  _createClass(Speaker, [{
    key: "speak",
    value: function speak() {
      console.log(this.message, this);
    }
  }]);

  return Speaker;
}();

var speaker = new Speaker("hello!");
speaker.speak();

/***/ })
@javan
Copy link
Contributor Author

javan commented Jan 16, 2019

I'm guessing this change is the cause:

Separate rule to compile node modules (fixes cases where ES6 libraries were included in the app code) #1823

And that this rule is being applied to the application's .js:
https://github.com/rails/webpacker/pull/1823/files#diff-456094c8451b5774db50028dfecf4aa8

@javan
Copy link
Contributor Author

javan commented Jan 16, 2019

Confirmed. Removing that loader (environment.loaders.delete('nodeModules')) fixes it.

oleksii-leonov added a commit to oleksii-leonov/webpacker that referenced this issue Jan 16, 2019
New `nodeModules` loader significantly change behavior.
It's specified in **Breaking changes** section but description is quite small.

As example, @javan have issue with `nodeModules` loader (rails#1892). Also, in my build it breaks `mapbox-gl` package (mapbox/mapbox-gl-js#3422).

This PR adds more details to `CHANGELOG.md` about `nodeModules` loader and receipt to keep previous behavior.
@jakeNiemiec
Copy link
Member

jakeNiemiec commented Jan 16, 2019

Relevant article from Babel: https://babeljs.io/blog/2018/06/26/on-consuming-and-publishing-es2015+-packages

Edit to clarify: I was not in favor of forcing all .js files to compile to ES5. There’s safety in numbers and webpacker should cater to how packages were intended to be consumed.

@javan
Copy link
Contributor Author

javan commented Jan 17, 2019

I understand the motivation behind compiling .js in node_modules/, but the problem is that this loader applies to all .js files, including my application's. It ends up clobbering my desired babel.config.js settings.

@javan javan closed this as completed in c73dd7a Jan 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants