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

src: add process.versions.iojs #491

Closed

Conversation

othiym23
Copy link
Contributor

This gives user code a forward-compatible means for determining whether it's running in io.js or Node.js. Also, to ensure backwards compatibility, it leaves the existing process.versions.node alone.

This will be helpful for tools like node-gyp to figure out whether they're running against Node.js or io.js. See nodejs/node-gyp#564 for a discussion of how this might be useful.

Everything's basically the same except:

// % ./out/Release/iojs -p process.versions
{ http_parser: '2.4',
  node: '1.0.3',        // <-- not removed
  iojs: '1.0.3',        // <-- added
  v8: '3.31.74.1',
  uv: '1.2.1',
  zlib: '1.2.8',
  ares: '1.10.0-DEV',
  modules: '42',
  openssl: '1.0.1k' }

PR: @rvagg
PR: @bnoordhuis

This gives user code a forward-compatible means for determining whether
it's running in io.js or Node.js. Also, to ensure backwards
compatibility, it leaves the existing `process.versions.node` alone.
@cjihrig
Copy link
Contributor

cjihrig commented Jan 18, 2015

I'm +1 on this change, and LGTM. It might be worth referencing the previous discussion on #253.

@othiym23 othiym23 mentioned this pull request Jan 18, 2015
@othiym23
Copy link
Contributor Author

Thanks, @cjihrig. Left a comment there to try to persuade my boss that this PR is a good idea.

@rvagg
Copy link
Member

rvagg commented Jan 18, 2015

good luck on that, he's been the most vocal against it

@isaacs
Copy link
Contributor

isaacs commented Jan 18, 2015

Yeah, I guess nodejs/node-gyp#564 does present a worthwhile use-case.

Mostly, I'm just opposed to people using it, and I've been burned in the past creating API that I don't think people should use. I fear process.versions becoming the navigator.userAgent of SSJS runtimes.

What's wrong with just using process.execPath? Either it ends in "iojs" or it ends in "node", right? (I am afraid that the answer will be "because windows", but it's worth checking.)

If we're going to add a "brand name" to the API, I'd prefer it to be explicitly that, rather than tacking onto process.versions. Maybe process.releaseor something that would have uname-inspired info stuff? Then we could also maybe differentiate between stuff coming from the pkg, vs built from git, vs a specific PPA release or something. There've been quite a few debugging sessions where I would have loved to know that.

@rvagg
Copy link
Member

rvagg commented Jan 18, 2015

Existing detection mechanisms that I can think of that should be cross-platform safe:

Version

function isiojs () {
  return parseInt(process.version.match(/v(\d+)/)[1], 0) >= 1
}

This is semi-safe because nobody here believes that "Node.js" will reach 1.0. There is a desire on both sides for reunification and when/if that happens there won't be an overlapping release numbering. If there isn't reunification then what's the realistic path for joyent/node to reach 1.0 when it can't even get to 0.12?

NODE_MODULE_VERSION

function isiojs () {
  return parseInt(process.versions.modules >= 42 && process.versions.modules < 100)
}

This is semi-safe for similar reasons but has some additional possible benefits:

  • there's a lot more room between the current number that joyent/node has and what io.js has and it would take a lot of releases and a lot of breaking ABI changes to get to an overlap
  • it also doesn't conflict with atom because they have jumped to 100 as of a couple of days ago

Fork

function isiojs (callback) {
  require('child_process').exec(process.execPath + ' -h', function (err, stderr, stdout) {
    if (err) return callback(err)
    callback(null, /iojs\.org/.test(stderr.toString()))
  })
}

This is probably the safest option as it's unlikely Node.js will ever include that in its help output (unless "reunification" results in both projects still existing and we decide to use node -h to describe some of that??)

The main drawback is the poor performance and need for forking.

@rvagg
Copy link
Member

rvagg commented Jan 18, 2015

If we really must have a User-Agent equivalent then I'd like to propose a process.runtime string and encourage Atom and joyent/node to adopt it also. It's not essential that anybody else does but at least process.runtime==='iojs' gives us what we want.

@domenic
Copy link
Contributor

domenic commented Jan 18, 2015

nodejs/node-gyp#564 (comment) seems like the cleanest idea to me, FWIW.

I agree that this is going to head toward user-agent hell, i.e. io.js already has process.versions.node + process.versions.iojs; I imagine once code starts checking for and requiring process.versions.iojs, Node will be forced to add process.versions.iojs.

@bnoordhuis
Copy link
Member

I like @rvagg's process.release proposal (what @domenic links to), it kills two birds with one stone.

What's wrong with just using process.execPath? Either it ends in "iojs" or it ends in "node", right? (I am afraid that the answer will be "because windows", but it's worth checking.)

Because Windows. :-) I'm given to understand by @piscisaureus that the node.exe stub would have a process.execPath that ends in node, not iojs.

@othiym23
Copy link
Contributor Author

I agree that this is going to head toward user-agent hell, i.e. io.js already has process.versions.node + process.versions.iojs

Well, not yet – this patch still isn't merged, and I will cry only a few salty tears of bitterness if it doesn't get merged, because I spent all of 10 minutes putting it together in the first place.

I also see no reason why joyent/node would ever add a process.versions.iojs -- for the original use case I was envisioning, the value of process.versions.iojs is far less pertinent than its existence (really – you could just have it be a getter for Math.random() for all I care). It's simply there so tooling can have code like:

if (process.versions.iojs) {
  // do iojs-specific stuff
}
else {
  // do "regular" Node.js stuff
}

Really, there's only two questions that need to be answerable, IMO:

  1. Am I running in a Node-like environment?
  2. What kind of Node-like environment is it?

#493 is acceptable to me, except that it I still have to do RegExp matching against URLs in its current form if I want to know whether I'm running io.js brand node, or Node.js brand node.

As a side note, I feel like I'm missing something important. People claim that feature-detection isn't necessary, but if all I have is a runtime and / or V8 version, then I basically need to build some kind of kangax-style feature matrix corresponding to those versions. In what universe is this not a huge pain in the ass?

@rvagg
Copy link
Member

rvagg commented Jan 19, 2015

@othiym23 note my PR has a process.release.name property with "iojs", as long as we're providing urls with it we may as well go forward and just give a name. It might be good to encourage Atom (and NW.js?) to do a similar thing that we end up doing here.

@othiym23
Copy link
Contributor Author

@rvagg 👍

With that, I withdraw this PR in favor of that one, even though I personally think this solution is simpler than either process.release or putting everything in process.config, which is turning into sort of a grab-bag object.

@othiym23 othiym23 closed this Jan 19, 2015
@ruimarinho
Copy link

@othiym23, what I think @domenic was referring to was the path that some user agent vendors, specifically MS/IE, took to precisely evade those checks by tricking user agent string matches (due to the replacement of "MSIE" for "like Gecko") into recognising it as another browser. When/if Joyent finds enough value on "built for io.js" packages that use the process.versions.iojs check to determine whether they're compatible or not on the node runtime, they'll probably just add that property as an alias to node too when they feel prepared for it.

I may also not be getting the full picture, but I do believe some sort of feature detection will be needed if backwards compatibility is needed (e.g. use native Promises or fallback to a userland implementation) or a more convoluted syntax support test (eval), even though transpilers might play an important role there.

Even if developers find a specific need that does require knowledge about the runtime, in my opinion, we should focus on promoting use cases where they are actually necessary (as #493 shows). In fact, I'm surprised npmjs.com/is-iojs isn't already taken :-)

@rvagg
Copy link
Member

rvagg commented Jan 19, 2015

@othiym23 providing explicit URLs gives us immediate support for nightlies when this lands in node-gyp, there's also the fact that we've improved (IMO) on some conventions in /dist/ inherited from node so we're not 100% compatible.

@edef1c
Copy link
Contributor

edef1c commented Jan 21, 2015

I'm somewhat doubtful about the value of an explicit UA string style thing — feature flags / feature detection should take their place, imo. Is there any specific reason feature detection won't do, and if so, why is this considered a better solution than feature flags?

@dougwilson
Copy link
Member

If there is a breaking change in the API for require('url') between 1.0 and 2.0, how do I know which version I'm getting? And how do I, as a module author, say I only want the 2.x version of it?

@othiym23 othiym23 deleted the othiym23/process-versions-iojs branch February 26, 2015 20:58
@domenic
Copy link
Contributor

domenic commented Feb 26, 2015

@dougwilson you test to see which behavior you have and throw an error if you get the behavior you don't want.

@dougwilson
Copy link
Member

At that point, why do i even bother putting anything in my dependencies that is not just "*" and simply detect everything everywhere, always?

@dougwilson
Copy link
Member

I don't see why you would even bother with semver if I cannot use it.

@domenic
Copy link
Contributor

domenic commented Feb 26, 2015

Are you just trolling? Obviously your dependencies are different than the executable running them, in that you actually control (through package.json) which versions of your dependencies get installed.

@dougwilson
Copy link
Member

Right, but is require('url') not a dependency? I'm asking a serious question since November and it seems like no one gives two shits.

@dougwilson
Copy link
Member

@domenic can you help me with the best way to feature detect for 1738c77 ? So far I'm just adding an unnecessary require('_stream_wrap') in my file and letting it throw if it isn't there. Is it OK to require() underscore-prefixed modules?

@vkurchatkin
Copy link
Contributor

Is it OK to require() underscore-prefixed modules?

No, please, don't

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

Successfully merging this pull request may close these issues.

10 participants