Feature - scope babel 7 support

[noygal]

I've expended the options to support babel option, this options apply only to the transform function as the other two uses were removed for babel 7, this is not the best approach as we have two separate babel packages running in the package - babel-core for configuration setup and the optional babel for transform, but I think that the solution would come from a proper babel 7 integration.

For the time being, it is a pretty simple solution for early babel 7 adaptors.

Added:

• Test (basic - option passing and compile)
• devDependancies - for testing (@babel/core, @babel/preset-env)
• Documentation (FAQ section)

Update: I've noticed this PR - #244, and removed the babel-core dependancies.

[tex0l]

It seems I have a bug with this PR. Your delete opts.babel breaks babelify when there are multiple files to go through. The first file uses @babel/core, but the next ones use the default babel-core which breaks the whole interest of your PR ^^.

I worked around by modifying the following line:

  return function (filename) {
    var extname = path.extname(filename)
    if (extensions.indexOf(function (ext) {return ext === extname}) > -1) {
      return stream.PassThrough();
    }

    var _opts = sourceMapsAbsolute
      ? Object.assign({sourceFileName: filename}, opts)
      : Object.assign({}, opts); // <-- this was previously `opts`

    return new Babelify(filename, _opts);
  };

[arantes555]

Seeing the same problem as @tex0l . Another, cleaner option would be to modify the constructor as such :

function Babelify(filename, opts) {
  opts = Object.assign({}, opts); // <-- adding this
  if (!(this instanceof Babelify)) {
    return Babelify.configure(opts)(filename);
  }

  stream.Transform.call(this);
  this._data = "";
  this._filename = filename;
  this._babel = opts.babel || babel
  delete opts.babel
  this._opts = Object.assign({filename: filename}, opts);
}

so that the function does not have side-effects on its arguments, which is very ugly

[noygal]

@tex0l @arantes555 , missed that - I only work with single file.

I've added the bugfix, can you check it out?

[tex0l]

@noygal works for me :) thanks!

[loganfsmyth]

It seems like if we're going to update this for 7.x, we might as well change this to @babel/core directly as a breaking change.

[loganfsmyth]

I don't think this is a good idea. If we're going to support 7, we should just add it directly. The utilities that were removed can easily be implemented inside this module on their own, if we actually want to support them.

[noygal]

I didn't want to break the current implementation as there were breaking changes in babel 7 (everything became scope), maybe we can add it to the next major version.

[loganfsmyth]

It'd be great to update this to use the callback version that landed in beta.32. e.g.

babel.transform(this._data, this._opts, (err, result) => {

[noygal]

nice, but will need to check babel version before using the new API, I'll update it and run the tests.

[loganfsmyth]

Is this intended to be .filter?

[tex0l]

I believe it should be extensions.includes(path.extname(filename)), right?

[loganfsmyth]

Node 4 is still supported by Babel, so we'll want to stick to features that work there.

[tex0l]

That's what I was just checking...

[noygal]

that why I went with indexOf, I don't think there's any node/browser version that don't support that.

[arantes555]

@noygal except indexOf takes a value, not a function, as argument :/

[noygal]

😳 ...
didn't use for a while now

[niftylettuce]

Can we have this merged yet please? Thank you

[niftylettuce]

@noygal you need to update package.json - and remove babel-core and fix the peerDependencies

e.g. ladjs@a76a30c

[niftylettuce]

Also need to update the top of the file to use @babel/core instead of babel-core. The tests as well would need updated, e.g. ladjs@165d668#diff-168726dbe96b3ce427e7fedce31bb0bcR4

[hzoo]

Sorry for the super long delay 😅, going to go with #255 instead! Thanks again.