outmoded / hapi-contrib

Discussion forum for project contributors
Other
78 stars 25 forks source link

ES5 builds #67

Closed nlf closed 8 years ago

nlf commented 8 years ago

Since this has come up multiple times, I think it's time we start an organization wide discussion.

Do we, or do we not, bundle es5 builds of modules?

The current stance (and my personal opinion) has been no, since hapi is focused on server side development (and the latest node version) that's the use case we focus on.

If the modules happen to work in a browser, or it takes only minimal changes that don't impact server-side code to allow it to work in a browser, I'm fine with making those changes and I don't think any of the rest of us would object.

The issue here is in regards to the building and publishing of transpiled code. This is a common pattern for non-js languages (coffeescript, typescript, etc) to follow. Since our code is pure node and simply uses features that require a newer version, I would argue that transpiling is not our problem and is the burden of the developer who wishes to use the module in the browser.

Thoughts?

(edited to reflect the real issue, which is es5 builds and not browsers specifically)

mark-bradshaw commented 8 years ago

I don't have much to add other than general agreement. I see no reason to maintain alternate versions but I'd say individual module owners can decide it's worth it to them. On Sat, Nov 28, 2015 at 12:07 PM Nathan LaFreniere notifications@github.com wrote:

Since this has come up multiple times, I think it's time we start an organization wide discussion.

Do we, or do we not, bundle browser builds of modules?

The current stance (and my personal opinion) has been no, since hapi is focused on server side development that's the use case we focus on.

If the modules happen to work in a browser, or it takes only minimal changes that don't impact server-side code to allow it to work in a browser, I'm fine with making those changes and I don't think any of the rest of us would object.

The issue here is in regards to the building and publishing of transpiled code. This is a common pattern for non-js languages (coffeescript, typescript, etc) to follow. Since our code is pure node and simply uses features that require a newer version, I would argue that transpiling is not our problem and is the burden of the developer who wishes to use the module in the browser.

Thoughts?

— Reply to this email directly or view it on GitHub https://github.com/hapijs/contrib/issues/67.

Marsup commented 8 years ago

I think I expressed my views on many occasions. Against.

devinivy commented 8 years ago

I essentially agree with @nlf. My followup question is, is there any simple way we can empower community members to maintain a separate browser build of a hapi module if they choose to do so? I'm not talking about lead maintainers– I'm thinking of other community members who desire a browser version.

hueniverse commented 8 years ago

Unless the module is explicitly designed for client-side, no. This is more than just converting ES6 to ES5. We do not test on any browser, we use dependencies which can bring in a sizable codebase, and there are plenty of edge cases unique to the browser (an old one used to be usage or instanceof).

The exception is in modules like nes in which there is a client-side component that is designed to run on the client. Same with my own hawk and oz modules. In those cases I think providing a cross-platform solution for browsers makes sense.

Marsup commented 8 years ago

The question is actually a bit broader than that, some people are also asking for ES5 builds, as in 1-to-1 conversion of files to ES5, so they don't have to do it themselves. I'm also not in favor of that but that should be part of the debate as it involves a compilation step in both.

jasonswearingen commented 8 years ago

The reason why ES5 builds are useful is then people can use browserify to create browser bundles, and have it work in ALL browsers. Currently they (ES6 files) will only work well in Chrome, and using Babel is a bit of a nightmare (compile times are 20+ seconds, plus opaque runtime error)

Tangental, but I write ES6 via typescript, which automatically compiles to ES5 for me. I know a tooling change is something you guys would want to do, but just so you know it's an option ;)

jasonswearingen commented 8 years ago

If anyone wants to get hapi utils working in the browser, here's a high-level on how I got Wreck to work:

1) Use Browserify (obvious?) 2) Use the Closure Compiler, you can set it's options to not do any minification, just doing ES6 to ES5 translation. it works great. (Using it via gulp, the https://www.npmjs.com/package/gulp-closure-compiler module)

Note, I originally tried using Babel instead of Closure Compiler, but Babel takes a long time to run and it's resulting output of some of the Wreck dependencies end up with syntax errors somehow (debugging it's output is difficult). Switching to the closure compiler worked on the first try. Also, can use this as a minification step too.

EDIT: note that you don't need closure compiler to work in Chrome, but as Wreck/other hapi utils use ES6 code, it won't work in IE (or Safari I think). using closure for transpilation to ES5 makes it work (down to IE9 at least)

Marsup commented 8 years ago

I don't want to ship anything else than my package, pure and simple. In the not so far future, we'll switch to more ES6 features that are in node 6+ but not 4, I don't want to dumb it down to ES5 if your platform supports some ES6 features, that's the whole point of babel, even more since 6, adaptive build depending on your target, and that's on your end, not on the library creators.

If/when someone comes with a solution that allows me to ship a build "recipe" and not a pre-built package that works universally on all bundlers, I'll consider it, until then, that's a no for me.

jasonswearingen commented 8 years ago

@Marsup I was more saying "anyone who wants to get hapi utils working in the browser" to mean end-developers, sorry I was not clear on that: you are right that my workflow is certainly not suited to package devs!

However, If you want a solution for package devs, it's actually pretty simple: run your output package.js through closure-compiler as a post-build step, choosing to do only ES6 to ES5 transpilation via the --transpile_only option.

nlf commented 8 years ago

The end result of this issue is what sounds like a consensus that we have no intention of publishing transpiled modules.

If there's a simple config we can add to package.json to make it simpler for end users to do that transpiling themselves, I personally have no objection there, but I absolutely have no intent of including the transpiled code in the repos or in the published modules.

Marsup commented 8 years ago

@nlf I don't know about consensus since a minority of maintainers expressed theirs opinions but there's no pro.

@jasonswearingen I'd rather not have any hook that would force me to depend on any compiler, after all I'm not compiling anything, the end user is.

nlf commented 8 years ago

changed the title and text to reflect the real issue, which is es5 builds rather than browser builds.

AdriVanHoudt commented 8 years ago

I am not a maintainer but I am in favor of keeping the modules node/server focused, if small change in config can help browser or es5 build than I am ok with. Only problem is that the package.json might get bloated with configs for all possible transpilers

ljharb commented 8 years ago

Publishing anything but compiled ES5 is hostile to users and the ecosystem. qs is currently used across many node versions and, via browserify and equivalents, across many browser versions.

Why is it OK to for the user to browserify but not to transpile, you ask? browserify (and webpack, and other similar approaches) is a simple module transform that does not require the user to know any details about how the module is implemented, only that it conforms to the node module interface - however, code transformation (like ES6, for example) require that certain transpilability caveats be understood, which means that users need to have intimate knowledge of how qs is implemented in order to transpile correctly and avoid breakage.

At the very least, qs should have a "browser" field with browser-compatible, transpiled, browserify output. This will at least solve the browser use case, albeit unnecessarily excluding users of older node versions.

Ideally, however, a simple "prepublish" step would be included that results in the "main" pointing to transpiled ES5 content. If someone wants to use the ES6 directly, they can follow the convention many already do, and provide the source files to be directly imported - ie, require('qs/es6') or something.

mattapperson commented 8 years ago

@ljharb I think to argue that modules must support backwards comparability or else be "user hostile" is rude to maintainers and just flat out wrong. Also your assumption of needing to understand what's under the hood of a module to transpile is a massive exaggeration.

hueniverse commented 8 years ago

@ljharb our ecosystem is server-side node.

qs is no longer being maintained and will be soon removed from this organization. hapijs is a server focused organization and the decision to provide an ES5 client is completely up to the discretion of each maintainer. This only happens when a server side module includes some client-side code like the nes and tv modules.

We do not currently support running any of the hapijs modules in the browser. If you want to do that, you are on your own the the module maintainer has no obligation to assist or support in that. We have rejected requests to make many of our modules browser-friendly again and again.

You are welcome to request to take over the qs module and maintain it yourself as you see fit.

ljharb commented 8 years ago

@hueniverse if there's currently nobody planned to maintain it, i'd be happy to take it over and ensure it's compatible with as many environments as possible - most of my modules work down to node 0.4 and IE6 / ES3. Where would you recommend I request that?

@mattapperson I'm not intending to be rude to maintainers, but I do think maintaining a module implies an obligation to its users. Indeed, there are many cases where it doesn't matter what's "under the hood" - but, for example, node has broken const semantics currently (ie, try for (const x of [1,2,3]) { console.log(x); } as an example) and if a module has relied on that, and then is run in an environment where const follows the spec? That could be a silent failure, and is indeed a risk.

hueniverse commented 8 years ago

@ljharb awesome. https://github.com/hapijs/qs/issues/140

Marsup commented 8 years ago

Not to be rude either, but I do think using a module implies you read the constraints and use versions that are compatible with your target.

I mean, nobody EVER gave any project shit when it decided to drop support for old JVMs or .NETs or whatever, why is this happening at all in this community ? Yeah you can't always have the latest and greatest stuff if you're on an outdated platform, so what ? If you don't want to use a transpiler to deal with YOUR target, that's not the problem of the maintainers. (I'm beginning to get pissed off by this topic in general, nothing against you @ljharb)

jasonswearingen commented 8 years ago

A quick note, there are modern and current environments where es5 is required: PhantomJs and Browsers are the 2 I am intimately aware of.

Though keep in mind that there is a known workaround for dev-user: Use a tool to transpile. That is a little bit "hostile" in that it requires more work and knowledge on the part of the dev-user, (I personally spent a few hours fighting / troubleshooting my browser builds due to this)

Perhaps an alternative workaround to the issue is to communicate this to dev-users who encounter the problem? Perhaps a try-catch that can detect when running outside an ES6 environment and shows an exception text explaining what the problem is and potential workarounds.

AdriVanHoudt commented 8 years ago

@jasonswearingen This is easily solved if someone forked the module and added config for the transpiler. This way the whole community can work together on maintaining an es5 build for different transpilers/configs. I mean that is what the fork button is for right? Linking to these builds/modules in the readme of the original module would be trivial imo.

Marsup commented 8 years ago

Well you're trying to run a project in a hostile environment because it was not made for it, of course there's work involved, otherwise like we're constantly saying, revert to a version that works out of the box.

jasonswearingen commented 8 years ago

@AdriVanHoudt I think the main issue is that some of the maintainers don't want to put in the time/effort to supporting ES5, which is understandable as it is a bit tangential to the mission at hand.

As @ljharb is volunteering to maintain ES5 compatible builds, that sounds great to me! But if it becomes too troublesome I think a descriptive error message would be an adequate compromise.

AdriVanHoudt commented 8 years ago

@jasonswearingen that is why you (or anyone else besides the maintainer of the module) should for it and take it upon him/her to do it.

And qs is a special case since hapi will not use it anymore. Maybe we should also talk about what happens with it. I know @hueniverse probably won't care anymore but if qs's new maintainer starts to support es5/transpilers/... this issue will keep popping up on other repos with the argument that if qs can do it why not X? Just a thought.

hueniverse commented 8 years ago

qs is moving out of hapijs and will no longer be relevant to this conversation.

AdriVanHoudt commented 8 years ago

Oh ok that solves my question, thanks

jasonswearingen commented 8 years ago

It seems like there choice to use ES6 might have an impact on various embedded Systems.

For example, I see that Visual Studio 2015 ships with an embeded version of the gulp command-line, and it doesn't support ES6.

Failed to run "C:\repos\stage3\pjsc-www\Gulpfile.js"...
cmd.exe /c gulp --tasks-simple
C:\repos\stage3\nlib\node_modules\hapi\lib\index.js:5
const Server = require('./server');
^^^^^
SyntaxError: Use of const in strict mode.
    at Module._compile (module.js:439:25)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at Object.<anonymous> (C:\repos\stage3\nlib\dev\webserver.ts:10:19)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at Object.<anonymous> (C:\repos\stage3\nlib\dev\nlib.ts:22:24)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)

I don't actually use VS's embedded gulp, and the current version you install via npm works fine, but just another datapoint for you to consider.

skeggse commented 8 years ago

We don't specifically support gulp or other task runners - it's up to the developer to handle their own environment. Node's ecosystem moves fast, which is why one generally doesn't install Node from a package manager (aside from a Node-specific one), task runners should be no different.

AdriVanHoudt commented 8 years ago

I don't see how Hapi should take into account that VS ships with a embedded (outdated? I use gulp with es6) version of gulp?

jasonswearingen commented 8 years ago

Well, I am just suggesting that there is some tradeoff between adopting the newest features that Node.latest supports, and keeping the latest versions of the Hapi ecosystem compatible with other systems that lag behind.

Does the value provided by ES6 features outweigh compatibility concerns? If it is an explicit non-goal to consider these "legacy" runtimes then no problem! I am only posting because I love Hapi and have personally noticed it's incompatibility with 3 separate runtimes. (Browser, Node 0.12, VS Embeded Gulp).

Marsup commented 8 years ago

We never forced anybody to increase their versions, they can happily use 9.x versions if that's a hard constraint for them.

AdriVanHoudt commented 8 years ago

Hapi is a server framework, the fact that you can use it anywhere else is just a little extra you get from JS. If you choose to use it anywhere else then the server, it is your responsibility to make it work. That it worked before and not anymore is like blaming a lib for fixing a bug that causes your app to not work. You were relying on something that accidentally worked and was never the intention. As for node 0.12, node moves forward, Hapi moves with it. As @Marsup said you can still use 9.x (which has LTS if I am correct). Although I would suggest upgrading to a higher node version. (I also suspect VS to have an internal version of Node because I can't imagine Gulp breaking on it)

hueniverse commented 8 years ago

Here is the bottom line: hapi supports the same officially supported node distributions. Right now it's 4.x for LTS and 5.x for latest. That's it. Even the node core team does not support 0.x anymore. We do not use the latest features of ES6/7 unless they are available in node v4 because that's our baseline.

prashaantt commented 8 years ago

@jasonswearingen On a side note, you should take a look at VS Code that has excellent support for node debugging and ES6.

ljharb commented 8 years ago

@hueniverse I'm afraid that's incorrect - the node core team absolutely supports 0.10 and 0.12 until December 2016 according to the core contributor I just spoke to, and imo hapi should be too.

https://github.com/nodejs/LTS/pull/36

devinivy commented 8 years ago

hapi v9 is on 0.10 and will be supported until june or so, if I recall correctly. hapi doesn't support 0.12 for reasons unrelated to this issue.

hueniverse commented 8 years ago

We only support 0.x for limited security fixes via hapi@9.x.x. This will last until June (see https://github.com/hapijs/hapi/issues/2765). hapi never supported node 0.12. Support for node 0.10 ends in 10/2016 (so we are only 4 months off the official node team).

Anyone who does not upgrade their environment to node 4.x or later is stuck on the hapi 9.x ecosystem. That is never going to change. The 9.x ecosystem is, as has been made very clear repeatedly, is only getting critical security bug fixes.

This thread has never been about LTS support and critical security issues. It is about people who wants to install the latest versions and expect it to work. hapi has always been aggressive about breaking changes and upgrades requirement. For example, if we find a critical security issue in hapi >= v11, it will only be fixed in v12 (or whatever is latest).

MylesBorins commented 8 years ago

fwiw the node.js core support of v0.10 and v0.12 is solely LTS maintenance mode, meaning the only updates are security updates / severe maintenance updates. Seeing as how @hueniverse mentions only v9.x.x supports those versions I think worrying about node's support pre v4 is a bit of a moot point