Build should use different 6to5 configs for front-end scripts

UltCombo commented 9 years ago

The bug was introduced by 8062f95e3646d3d607a9bea2666252086eee08f9's usage of optional: ['selfContained'].

The issue is that src/bin/client/harmonic-client.js is being transpiled with the same settings as the Node.js scripts.

Possible solutions:

Make slush-es20xx's build.js plugin configs take an array of objects containing paths and options (perhaps this is too much grunt-like?)
Revert to compiling harmonic-client.js on every harmonic build instead of pre-compiling.

UltCombo commented 9 years ago

Guess I'll remove the ES6 syntax from harmonic-client.js to work around the issue for the time being, until slush-es20xx supports back-end and front-end transpiling in the same project.

UltCombo commented 9 years ago

Lowering priority now that an workaround has been implemented -- b040ba46b3c3af53d35ba9248091cf32fca12a9b

UltCombo commented 9 years ago

@jaydson perhaps you can provide some ideas here? Let me explain the history behind this:

Three months ago or so, the harmonic-client.js file was shipped as ES6 code and harmonic build would inline the site data inside harmonic-client.js and transpile it to ES5.

This means Harmonic had a direct dependency on the transpiler (Traceur back then) as harmonic build had to compile the harmonic-client.js file. But I've noticed it was unnecessary as we can pre-compile the harmonic-client.js file before publishing, and harmonic build only needs to inline the site data then.

So far, harmonic-client.js was being pre-compiled together with all other js files in the project. However, harmonic-client.js is targeted at a different environment (browsers) than the other scripts (Node.js). There are issues with this approach: browsers don't support CommonJS, and they would require 6to5's browser polyfill which we weren't including. This hasn't resulted into an actual issue before because of harmonic-client.js's simplicity -- it has no import/export, and no features that would require the polyfills.

The best approach would depend on what we are planning for the future:

If we just want to expose the Harmonic site data, we can just expose an object literal in the global scope and forget about transpiling.
If harmonic-client.js will become more complex, then we need different settings for compiling back-end and front-end scripts in the same project (this should be done in slush-es20xx).
If Harmonic will also transpile scripts inside templates, then we should move the transpiling back to harmonic build and re-add a direct dependency on a transpiler (6to5-core).

So, what is our use case @jaydson?

jaydson commented 9 years ago

I bet on Harmonic taking care of JavaScript both on client and server. I'm looking forward the new version of slush-es20xx.

One thing we need to do is unlink templates from Harmonic. We need to provide tools for developers build templates.

UltCombo commented 9 years ago

The problem is, if we centralize all pre-processing in the Harmonic core, then it will be a pain to maintain.

Right now we have Stylus and Less preprocessors built in the core (I believe there's an open issue for Sass as well), then if we're going to pre-process JS as well we will have to add 6to5 (and perhaps Traceur if users want to use it as well), and that would require setting requirements on the directory structure of the templates. This breaks separation of concerns and is not pluggable at all.

Perhaps we should move all template pre-processors to Harmonic plugins? @jaydson

jaydson commented 9 years ago

Yep, that's a problem to solve. I think the best way to deal with this situation will be have a good plugins structure. With this, we can move preprocessors and all related to 3rd party plugins.

For 6to5 i think we need to work differently, because we're using it in Harmonic's core.

UltCombo commented 9 years ago

@jaydson yes, the whole core is transpiled with it. Thing is, the only file in the core which is supposed to run in a browser is harmonic-client.js right now, whose contents could be entirely replaced with var Harmonic = /*drop JSON-encoded site data object here*/;

jaydson commented 9 years ago

Agree. We can do it by now. Let's keep it simple, and in the near future we can add full client-side-es20xx code.

UltCombo commented 9 years ago

:+1:

UltCombo commented 9 years ago

So should we just set a global variable instead of the class? This will be a breaking change btw.

jaydson commented 9 years ago

Yes, but we already have this, right?

UltCombo commented 9 years ago

@jaydson well, nope, right now harmonic-client.js contains an Harmonic class.

UltCombo commented 9 years ago

Fixed by c4753229329ff7db9a8d44de3f3ad8c5ed0152b3

JSRocksHQ / harmonic

Build should use different 6to5 configs for front-end scripts #113