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

Proposal: Harmony enabled binary, packages. #90

Closed
rektide opened this issue Dec 5, 2014 · 9 comments
Closed

Proposal: Harmony enabled binary, packages. #90

rektide opened this issue Dec 5, 2014 · 9 comments

Comments

@rektide
Copy link

rektide commented Dec 5, 2014

Greetings.

Some of us really want nothing to do with the old ES5 world and are wanting to make a clean hard break. I would like to see a node binary where harmony is the default. This could be a configuration variable at build time, it could be a node-harmony binary, or perhaps we're ready to throw the switch and just do it for everyone: something to avoid the sys-administration of wrapper shell scripts or project configuration needed to add --harmony.

For responsibility/compatibility sake, npm packages need some what to discern the target: engines might stand to have a harmony target that designates that a harmony targeting engine is needed.

Follow-up item: whether it's desirable or needed to have a --disharmony.

@bnoordhuis
Copy link
Member

You can bake extra V8 options into the binary with ./configure --v8-options="--expose_gc --harmony". I don't think we'd ever want to enable it in releases, though.

See nodejs/node-v0.x-archive#6999, that's a trial PR I opened to get the discussion going. The conclusion was that --harmony features are generally half-baked (if not outright buggy) and that it's generally better to let them mature until they are no longer behind a flag. Generators and promises are examples of that.

--disharmony

--noharmony should work.

@aredridel
Copy link
Contributor

A completely different way to approach this -- to declare a dependency on a harmony supporting binary -- is to bring that binary in yourself:

npm install --save [email protected]

Then --harmony will work for things run within your project via npm scripts.

Not exactly what you're asking, but has some of the same effects: declare what you're doing, and make it Just Work.

@caineio
Copy link

caineio commented Dec 8, 2014

Hello!

I am pleased to see your valuable contribution to this project. Would you
please mind answering a couple of questions to help me classify this submission
and/or gather required information for the core team members?

Questions:

  1. Issue-only Does this issue happen in core, or in some user-space
    module from npm or other source? Please ensure that the test case
    that reproduces this problem is not using any external dependencies.
    If the error is not reproducible with just core modules - it is most
    likely not a io.js problem. Expected: yes
  2. Which part of core do you think it might be related to?
    One of: debugger, http, assert, buffer, child_process, cluster, crypto, dgram, dns, domain, events, fs, http, https, module, net, os, path, querystring, readline, repl, smalloc, stream, timers, tls, url, util, vm, zlib, c++, docs, other (label)
  3. Which versions of io.js do you think are affected by this?
    One of: v0.10, v0.12, v1.0.0 (label)

Please provide the answers in an ordered list like this:

  1. Answer for the first question
  2. Answer for the second question
  3. ...

Note that I am just a bot with a limited human-reply parsing abilities,
so please be very careful with numbers and don't skip the questions!

In case of success I will say: ...summoning the core team devs!.

In case of validation problem I will say: Sorry, but something is not right here:.

Truly yours,
Caine.

Responsibilities

  1. indutny: crypto, tls, https, child_process, c++
  2. trevnorris: buffer, http, https, smalloc
  3. bnoordhuis: http, cluster, child_process, dgram

@cjihrig
Copy link
Contributor

cjihrig commented Dec 8, 2014

Closing based on @bnoordhuis response.

@cjihrig cjihrig closed this as completed Dec 8, 2014
@ruimarinho
Copy link

@bnoordhuis since V8 has introduced an es_staging flag to enable experimental (i.e. completed but not fully tested) features, the harmony flag will now only enable completed features. Theoretically, this makes it acceptable for node to be bundled with --harmony enabled on the release build considering the latest V8 version being shipped, specially since there is no evidence that this flag will go away when work on ES7 begins.

@rvagg
Copy link
Member

rvagg commented Jan 6, 2015

do we have a catalogue of what --harmony currently enables?

@ruimarinho
Copy link

@rvagg: yes - on master (3.31.x) and on 3.30.37.

@bnoordhuis
Copy link
Member

@ruimarinho Close but no cigar. :-) In V8 3.30 (what currently ships with io.js), --es_staging is --harmony minus --harmony_scoping; in 3.31, they are synonyms.

I personally don't really object to turning on --es_staging; in 3.30, you get the harmony string methods, like String#startsWith(), which is nice.

@ruimarinho
Copy link

@bnoordhuis ah, yes, just noticed that there was an update on that. Seems like --es_staging is a safe default for 3.30 then. Do you think that if it ends up being enabled on 1.0.0, would it make sense to keep the flag also enabled for later upgrades of V8 too (e.g. 3.31), now that there is a clear distinction between "in progress" and "completed" features?

Looking back at node 0.11.14 (V8 3.26.33), the only feature under staging was harmony_maths with pretty much everything else being a mixture of "in progress" and "completed" features, so I believe it was a reasonable decision at that time to not ship harmony by default.

kunalspathak added a commit to kunalspathak/node that referenced this issue Feb 26, 2017
boingoing pushed a commit to boingoing/node that referenced this issue Apr 6, 2017

* napi: Separate API for external ArrayBuffer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants