Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restrict code to ES5 #9

Closed
saghul opened this issue Jun 12, 2016 · 45 comments
Closed

Restrict code to ES5 #9

saghul opened this issue Jun 12, 2016 · 45 comments
Labels

Comments

@saghul
Copy link

saghul commented Jun 12, 2016

Great progress Fedor!

If this tool is to be used for compiling Node, I'd say it should be written without any ES6 features, so Node 0.10 can be used to build the latest Node.

Ideally with no dependencies. If someone is starting from zero, something like a small bootstrap VM built using Duktape could be used.

@indutny
Copy link
Owner

indutny commented Jun 12, 2016

Thank you, Saul!

I was going to use Babel for this purpose, and duktape is in my plans
indeed! 😉

On Sunday, June 12, 2016, Saúl Ibarra Corretgé notifications@github.com
wrote:

Great progress Fedor!

If this tool is to be used for compiling Node, I'd say it should be
written without any ES6 features, so Node 0.10 can be used to build the
latest Node.

Ideally with no dependencies. If someone is starting from zero, something
like a small bootstrap VM built using Duktape could be used.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#9, or mute the thread
https://github.com/notifications/unsubscribe/AAOjw7TJqCIAaSyyXYvoHtlT04AqMC4Oks5qLIO2gaJpZM4Iz5gW
.

@tejasmanohar
Copy link

@saghul I'm not @indutny, but personally, I don't think that's necessary. If someone is starting from point zero today, they should be using a later version of Node.js. I suggest we keep things moving forward so long as it works without flags on LTS.

@tejasmanohar
Copy link

@indutny Interesting. Babel works, too.

@kzc
Copy link

kzc commented Jun 12, 2016

Perhaps a minimal subset of fs, util, process, path and child_process can be made for duktape.

@indutny Is all this code all synchronous or are there any async parts relying on an event loop?

@indutny
Copy link
Owner

indutny commented Jun 12, 2016

@kzc no async parts. I'm going to isolate all node.js bindings into a separate JS file, so it would be easier to shim them.

@indutny
Copy link
Owner

indutny commented Jun 12, 2016

Some of them could be take from browserify, obviously. Like util.

@kzc
Copy link

kzc commented Jun 12, 2016

For ES6 to ES5 conversion buble has fewer dependencies and is much faster than babel. Mind you, it supports just a subset of ES6.

https://gitlab.com/Rich-Harris/buble

indutny added a commit that referenced this issue Jun 12, 2016
Put all node.js related "bindings" into `bindings.js` to make it easier
to shim them with duktape.

See: #9
@indutny
Copy link
Owner

indutny commented Jun 13, 2016

@kzc I'm not sure if dependency count matters that much, considering that this step will happen only on duktape builds.

@kzc
Copy link

kzc commented Jun 13, 2016

I guess if gyp.js is translated to ES5 and browserify'd for duktape and distributed with node in that form then it doesn't matter whether babel or buble is used to make it.

@indutny
Copy link
Owner

indutny commented Jun 13, 2016

There is no point in distributing it in ES5, ES6 is just fine for the latest node.

@kzc
Copy link

kzc commented Jun 13, 2016

ES5 gyp.js could be used for backports to bootstrap node itself.

@indutny
Copy link
Owner

indutny commented Jun 13, 2016

@kzc true, though not too much reason to do it with older versions of node.js . So ES5 IMO should be used only to provide a binary with duktape.

@kzc
Copy link

kzc commented Jun 13, 2016

ES5 gyp.js with duktape + bindings shim could bootstrap node itself with only a C++ compiler.

@indutny
Copy link
Owner

indutny commented Jun 13, 2016

Yeah, I agree. This is reflected here: #4

@saghul
Copy link
Author

saghul commented Jun 13, 2016

My point here was twofold: a) same as @kzc, but b) it should probably be possible to build Node with whatever (reasonable) Node in the PATH already. For b) that's 0.10 on Debian stable.

What are the ES6 features a (relatively) small library like this can't live without? IMHO it's simpler to not have to even transpile the code.

@tejasmanohar It's not reasonable to tell someone: "hey, download Node 4 in binary form in order to be able to compile Node 6". Node should be able to self-bootstrap.

@tejasmanohar
Copy link

@saghul Ah, I missed the original concern- self-bootstrapping. If that's the case, I'd also suggest avoiding transpilation just because I don't think it offers major benefits.

@indutny
Copy link
Owner

indutny commented Jun 17, 2016

I'm inclined to close this issue as a won't fix. Any objections?

@indutny
Copy link
Owner

indutny commented Jun 17, 2016

My arguments are the same just in case, transpiling should help us move forward for now, and we are not even sure if this tool will be distributed standalone at this point.

@RangerMauve
Copy link

I'm thinking that if anything, transpiling could be added once people actually start using this in the wild with older node versions or whatever else. I doubt it'd be hard to add in, and I also doubt that switching back to ES5 would be too difficult if it was really necessary.

@indutny
Copy link
Owner

indutny commented Jun 17, 2016

Ok, let's keep this issue open then. Valid argument.

@RangerMauve
Copy link

Well, I was actually thinking that it'd be OK to close it for now and then re-open once somebody really needs it. But that works too. 👍

@saghul
Copy link
Author

saghul commented Jun 18, 2016

✋ I object :-) That means we'd have to bundle the transpiler, plus the dependencies this thing already has 😭 making this whole thing a lot bigger than it really needs to be IMHO.

Also, I'm not sure if you considered it (or if it ever was one of your goals), but this tool could be used by others also wanting to dump the Python dependency GYP currently has implies.

@pmed
Copy link
Contributor

pmed commented Jun 18, 2016

Sticking to ES5 would allow to use gyp.js in node-gyp. Actually, I only see few ES6 features (function default parameters and destructuring assignment) in the gyp.js code that do not work in older stable Node.js versions.

I tried gyp.js with Node.js 4.4.5 in a branch es5, and it worked well.

@saghul
Copy link
Author

saghul commented Jun 18, 2016

@pmed 4.4 might still be too recent. 0.10 will be EOL in october and it's packaged in Debian Jessie (stable) so I'd hope it works there.

@kzc
Copy link

kzc commented Jun 18, 2016

If an ES6 gyp.js is transpiled to ES5 and browserify'd and bundled with node releases in that form it could be workable with duktape - but it's not ideal.

@indutny
Copy link
Owner

indutny commented Jun 18, 2016

@saghul sticking to v0.10 means:

  • no arrow functions
  • no const/let
  • no default argument values (I guess we can live without it)

@ibc
Copy link

ibc commented Jun 18, 2016

Please, just use ES6 and avoid Babel.

@saghul please, forget node 0.10 and let's move forward.

@kzc
Copy link

kzc commented Jun 18, 2016

@ibc Duktape only supports ES5. This can remove the python dependency on bootstrapping node.

@ibc
Copy link

ibc commented Jun 18, 2016

@kzc Duktape should update. ES6 is one year old.

@kzc
Copy link

kzc commented Jun 18, 2016

Duktape should update. ES6 is one year old.

That's not practical.

@indutny
Copy link
Owner

indutny commented Jun 18, 2016

@ibc pointless, though? Why should they emulate stuff that can be shimmed/transpiled?

@hzoo hzoo mentioned this issue Jun 19, 2016
@saghul
Copy link
Author

saghul commented Jun 19, 2016

@indutny all of the things you mentioned sound like minor inconveniences here, AFAIS.

@ibc I like ES6 as the first guy, but I'm trying to suggest a pragmatic approach here. If I want to deploy Node vX to production which uses this new gyp.js I should be able to bootstrap it without third-party binaries. If my distribution ships Node 0.10 it would be ideal if we can use that, falling back to a Duktape based tiny bootstrap interpreter. What you suggest is not reasonable, Duktape is not going to get full ES6 support overnight.

@kzc
Copy link

kzc commented Jun 19, 2016

If ES6 speed relative to ES5 is a concern:

https://kpdecker.github.io/six-speed/

@indutny
Copy link
Owner

indutny commented Jun 19, 2016

I have restricted the code to the limited ES6 subset: only const and let. It appears to be working on v4, v5, v6 node.js at the time.

Still, I kind of like the fact that it could be checked for const-ness with the help of eslint. Should we leave it this way and just use Babel for duktape?

@ibc
Copy link

ibc commented Jun 19, 2016

Node 0.12 does not support let. So there are 3 choices IMHO:

  1. Drop ES5 support (bye ES5 and Duktape).
  2. Use Babel so npm i gyp.js installs a just ES5 library.
  3. Publish a separate gyp.js-es5 NPM package.

@saghul
Copy link
Author

saghul commented Jun 19, 2016

@ibc How do you plan on running npm if you don't have Node built yet? It's a chicken-and-egg problem, we're talking bootstrapping here.

@saghul
Copy link
Author

saghul commented Jun 19, 2016

@indutny If you decide to go the Babel route it doesn't really matter how much ES6 you use anyway, does it? :-)

@indutny
Copy link
Owner

indutny commented Jun 19, 2016

@saghul well, at least this code will work without transpiler on v4 node.js

@ibc
Copy link

ibc commented Jun 19, 2016

we're talking bootstrapping here.

I thought the main purpose of gyp.js was to remove the Python dependency for building Node native modules (rather than building Node itself).

@indutny
Copy link
Owner

indutny commented Jun 19, 2016

@ibc we haven't decided yet 😢

@ibc
Copy link

ibc commented Jun 19, 2016

we haven't decided yet

Then I suggest deciding "what" prior to "how" :)

@indutny
Copy link
Owner

indutny commented Jun 19, 2016

Good idea!

@ibc ibc mentioned this issue Jun 20, 2016
@ibc
Copy link

ibc commented Jun 20, 2016

@indutny I have taken the liberty of suggesting the list of use-cases in #19, so this issue can focus on the original subject ("restrict code to ES5").

@indutny
Copy link
Owner

indutny commented Jun 20, 2016

@ibc thank you!

@indutny
Copy link
Owner

indutny commented Mar 23, 2017

Closed by: https://github.com/indutny/dukgyp

@indutny indutny closed this as completed Mar 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants