sintaxi / terraform

Asset pipeline for the Harp Web Server.
102 stars 101 forks source link

Babel Support #128

Closed jimjkelly closed 8 years ago

jimjkelly commented 8 years ago

This PR seeks to provide a another attempt at Babel support, in this case taking advantage of the fact that Babel out of the box does nothing, and leaves configuration to the user.

Like #82, this implements Babel through a JavaScript processor, but the difference is it does not implement it for .es files, but instead replaces the default js processor with one that always gets run through Babel. As mentioned, this does nothing by default, which means users should notice no difference.

With the addition of a .babelrc file in the directory they run harp from as well as installation of requisite packages, the user can activate Babel's features - which means they can do anything ES6 support, JSX support, the whole bag.

Some notes:

A simple example is illustrative.

$ harp init .
$ npm init  # Create a simple package.json
$ npm install babel-preset-react react react-dom

Add the following to .babelrc:

 {
    "presets": [
      "react"
    ],
}

Now, edit _layout.jade:

doctype
html
  head
    link(rel="stylesheet" href="/main.css")
  body
    != yield
    script(src="/example.js")

Edit index.jade:

h1 Welcome to Harp<span id="babel"></span>.
h3 This is yours to own. Enjoy.

Finally you can try it without Babel, this will work without the .babelrc - add an example.js with:

document.getElementById('babel').innerHTML = ' (without Babel goodness)'

Run Harp, you should see the expected page. Now try some simple React in example.js instead (ensure your .babelrc is set up):

var React = require('react')
var ReactDOM = require('react-dom')

var Goodness = React.createClass({
  render: function() {
    return <em> (with Babel goodness)</em>;
  }
});

ReactDOM.render(
    <Goodness />,
    document.getElementById('babel')
)

That should do it.

jimjkelly commented 8 years ago

As a follow up - I've been using this on my own over the past week or so for React (JSX) and ES6 transpiling, and it has mostly worked well. The one aspect that doesn't work well is that using the new ES6 import stuff, you'd hope when that gets transpiled to a require statement that the Browserify step in Harp would handle reconciling that, but it doesn't.

In the event there ends up being interest in this - would it make sense to Browserify the results of any of the js processors after they run, instead of before?

DawnPaladin commented 8 years ago

I would be so happy if there was an easy way to use ES6 in Harp :grin: Hope this PR gets accepted.

lunelson commented 8 years ago

@jimjkelly

EDIT -- sorry, obviously you are using it with the browserify feature if you have require statements; I'm not sure what I'm doing wrong but it chokes every single time with new JS_Parse_Error. The babel step does not seem to happen, for some reason. I will try again from a blank slate I think

lunelson commented 8 years ago

Just tried applying this PR to a fresh copy of @sintaxi's master branch, it throws JS_Parse_Error on the following

console.log(`this is a template string`);

... with this in .babelrc

{
    "presets": [ "es2015" ]
}

Any ideas?

lunelson commented 8 years ago

Never mind, solved it. Local package configuration problem 😅. WRT your question above concerning at which step to transform, I can't imagine why there would be a need to transform after the bundle step; but possibly, the es6 imports are not working here because it's babel-core on the raw JS file, rather than the babelify transform step with browserify? Or maybe it's because browserify is only being processed on some files (when a require() statement is detected)? In any case, given the latter, I think passing all files through babel whether browserified or not is the right choice for now, but perhaps the ultimate solution requires a better coordination of these two steps

jimjkelly commented 8 years ago

Yeah I guess I'm curious if there's interest in this being merged from the maintainers? I'm continuing to use it daily for my work on my end, but resolving the ES6 import issue is pretty high on my list of priorities.

lunelson commented 8 years ago

I wish I was more expert in how browserify and babel work together; babel on its own (based on what I can see using babeljs.io/repl) does transpile es6 import statements to require but there are so many ways an import statement can work, I'm sure it requires parsing the imported file as well, to make assignments correctly. I'm guessing this requires using the babelify transform inside the browserify process.

There is a helper function in terraform called needsBrowserify which at the moment is keying on the following Regex: /^[^#\/'"*]*(require|module|exports)\b/m—so, only looking for CommonJS syntax—I would suggest maybe forcing this to always be true, or simply removing the check, and running all JS through browserify with a babelify transform, and removing the babel transform you've put in to the js compiler. In principle, your proposal would still apply (reference .babelrc; do nothing by default); but bringing the babel transform inside the browserify step would allow (in theory) the import functionality

jimjkelly commented 8 years ago

Yeah I too have been stumbling through this as I started using React and JSX this year, trying to learn how these tools interoperate.

And yeah I don't recall what made me shy away from using the Babelify plugin for Browserify - but actually that might work well. I don't think that makes use of the .babelrc file by default, but perhaps we could either code up that inclusion to the runtime configuration ourselves indirectory, or use some other simple method.

Because otherwise I do think the JSX/React type transforms would have to happen before the ES6 import stuff - beause for instance if you tried to import a React class I think it'll choke on the syntax.

But I'll look at maybe changing this to that method and see how it works, because that should keep it all at the same time.

lunelson commented 8 years ago

Cool. Yeah I'll be interested to know how you get on. As far as I can tell with babelify's documentation, it passes its options on to babel so it might be possible to configure it the same way as you've done, i.e using .babelrc; however caveat seems to be that some options must be specified twice, once to browserify and then to the transform e.g. with extensions. Good luck!

lunelson commented 8 years ago

@jimjkelly also ping me if I can help in any way

jimjkelly commented 8 years ago

So, the approach works well for its own purposes, but it does seem to introduce some incompatibilities with the Coffee script processor, or at least it makes some tests fail. Additionally, the minification in the render call seems to make some JS tests fail.

I'd really kind of hoped to be able to at least add this in a non-breaking way to current Terraform, but it seems that isn't going to be easy.

My personal recommendation would be to remove the processors idea altogether (and coffeescript processing out of the box, I suppose) and rely on Browserify/Babel. Obviously that would be a breaking change, so couldn't go in until another major revision. The (slight) problem with that is that there doesn't currently seem to be a transform for CoffeeScript, though I don't see a reason one couldn't be made.

If there's interest in this, I may maintain a fork of Terraform/Harp that has these ideas implemented with the goal of having it go into the next major release. If there's not interest, well I may maintain a fork forever. :/ Certainly having the JSX/ES6 transforms have become integral to my workflow.

lunelson commented 8 years ago

Hey @jimjkelly glad to hear you've been working on this; there is in fact a transform for coffeescript: it was originated by the author of browserify and now maintained here

jimjkelly commented 8 years ago

Ah, so there is! I was looking for something in Babel but I guess that works too. I integrated, and as best as I can tell it largely works, though there are several tests failing for reasons that are not immediately obvious. I need to get back to some productive work, but I'll probably push this up as a fork shortly and then keep chipping away at it.

One question that pops up - this obviates the need for the processors, but I guess keeping them around, perhaps even as empty files, would be good to maintain consistency. I know a few places in the code this is checked to build a list of supported extensions.

lunelson commented 8 years ago

yeah my sense of how terraform works is that they need to be there; but could simply pass through their files unchanged, in this scenario

jimjkelly commented 8 years ago

I'm not really working on this branch anymore and given there's no upstream interest I'll go ahead and close this.

lunelson commented 8 years ago

Dude, it's a bummer, but I can tell you I've made some progress rolling my own compile-on-the-fly dev-server solution, and you can build something faster and more lightweight than this. It's WIP but if you're interested I'll send it to you

jimjkelly commented 8 years ago

Yeah I'd be curious to see what you came up with. My email is on my github profile. For me I moved more towards a more traditional model of managing these various components myself, first in npm scripts and have recently been messing with webpack, trying to get some of the HMR goodness going (though it's a bit buggy).