Use babel-preset-latest

[gnestor]

I’m submitting this and opening it up for discussion.

• Node 6 supports 99% of es2015. babel-preset-latest includes es2015, es2016, es2017. I think that jp-babel should provide support for these by default but I acknowledge that there might be issues in doing so.
• I added a “preinstall” script that should delete the node_modules directory before installing, so that upgrading doesn’t cause conflicts (e.g. I upgraded the babel dependencies without removing node_modules and got a You gave us a visitor for the node type "ForAwaitStatement" but it's not a valid type error). This should also address https://github.com/n-riesco/ijavascript/issues/76 regarding version mismatch of Node bindings for zmq (which require a rebuilding if the Node environment version changes).

[n-riesco]

It would've been better to submit this commits in different PRs (since they address independent issues).

Here's my review:

• https://github.com/n-riesco/jp-babel/pull/9/commits/848c1f6f404b3f297541974b82ee43934561d422 is unnecessary 6 and ^6 are equivalent. See the version calculator here.
• https://github.com/n-riesco/jp-babel/pull/9/commits/8167477cb790ed4984582ab0dfb1857a7848738e only works if you installed jp-babel using git clone. I also have the following arguments against the idea of wiping node_modules:
  • babel is a huge dependency (and thus, slow to install)
  • node_modules is npm's responsibility (messing with node_modules should be a last resort).
  • The way I'd like to address this issue is with a try-catch block and throwing and error message that tells the users how to solve it (ideally, in a cross-platform way).
• https://github.com/n-riesco/jp-babel/pull/9/commits/a93f3e3ec3b95ecaedd8e3b0d56aca2515a524bf Other than the size of the babel dependency, what would the issues of defaulting to babel-preset-latest?

[gnestor]

I just removed the other commits regarding dependency versions and preinstall script.

848c1f6 is unnecessary 6 and ^6 are equivalent. See the version calculator here.

Ok, thanks for clarifying.

The way I'd like to address this issue is with a try-catch block and throwing and error message that tells the users how to solve it (ideally, in a cross-platform way).

This approach sounds good to me, except that the error I encountered (You gave us a visitor for the node type "ForAwaitStatement" but it's not a valid type) was a babel compilation error (that I saw in the output of a cell), so I don't know how easy it would be to catch that, let alone dynamically suggest a solution. Although I did a git clean -xfd && npm install to resolve it, I bet an npm update would've resolved it too. Regarding Node bindings issue with zmq, it looks like jmp now depends on zmq-prebuilt, so this issue is resolved now?

a93f3e3 Other than the size of the babel dependency, what would the issues of defaulting to babel-preset-latest?

I don't see any issue myself, I just wanted to see what others thought about it, to be safe.

[n-riesco]

On 02/11/16 19:54, Grant Nestor wrote:

The way I'd like to address this issue is with a try-catch block and throwing and error message that tells the users how to solve it (ideally, in a cross-platform way).

This approach sounds good to me, except that the error I encountered (|You gave us a visitor for the node type "ForAwaitStatement" but it's not a valid type|) was a babel compilation error (that I saw in the output of a cell), so I don't know how easy it would be to catch that, let alone dynamically suggest a solution. Although I did a |git clean -xfd && npm install| to resolve it, I bet an |npm update| would've resolved it too.

Could you open an issue for this (so I don't forget)?

Regarding Node bindings issue with zmq, it looks like jmp now depends on zmq-prebuilt, so this issue is resolved now?

No, it doesn't solve the issue (it compiles zmq statically, but the issue is with Node's V8).

Kyle mentioned https://github.com/juliangruber/require-rebuild may help.

a93f3e3 <https://github.com/n-riesco/jp-babel/commit/a93f3e3ec3b95ecaedd8e3b0d56aca2515a524bf> Other than the size of the babel dependency, what would the issues of defaulting to babel-preset-latest?

I don't see any issue myself, I just wanted to see what others thought about it, to be safe.

OK. I'm merging the PR.

[Announcement]

update this to babel-preset-env