Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Added a command-line option to bin/express to generate engine entries for node and npm in package.json #1486

Closed
wants to merge 1 commit into from

4 participants

@curious-attempt-bunny

This is a simplified version of a prior pull request based on feedback: #1483

@shesek What do you think?

@tj
tj commented

hm.. heroku needs both?

@shesek

@curious-attempt-bunny Looks great, but perhaps its better to use '~'+process.versions.node rather than hardcoding 0.8.x? I also tried to check how to get npm's version, but it seems like you can't require() it unless you install it locally, so I'm not quite sure how to go about that.

@visionmedia You mean both node and npm versions? I just tried to push to Heroku without an npm version (but with node set to v0.8.x):

-----> Node.js app detected
-----> Resolving engine versions
       Using Node.js version: 0.8.14
       Using npm version: 1.0.106
-----> Fetching Node.js binaries
-----> Vendoring node into slug
-----> Installing dependencies with npm
       Error: npm doesn't work with node v0.8.14
       Required: node@0.4 || 0.5 || 0.6
           at /tmp/node-npm-9SVb/bin/npm-cli.js:57:23
           at Object.<anonymous> (/tmp/node-npm-9SVb/bin/npm-cli.js:77:3)
           at Module._compile (module.js:449:26)
           at Object.Module._extensions..js (module.js:467:10)
           at Module.load (module.js:356:32)
           at Function.Module._load (module.js:312:12)
           at Module.require (module.js:362:17)
           at require (module.js:378:17)
           at Object.<anonymous> (/tmp/node-npm-9SVb/cli.js:2:1)
           at Module._compile (module.js:449:26)

It uses npm v1.0 by default, which only works with node <=v0.6. Seems like it needs both of them.

@tj
tj commented

oh damn, Ideally we just set them without the flag IMHO, I'm not sure people will even notice it's there otherwise, I definitely wouldn't guess it's related to heroku etc, so I would probably just end up adding them manually

@curious-attempt-bunny

@shesek I like sniffing the node version and assuming that. If I go with that do you see a problem with assuming npm 1.1.x?

@visionmedia Sure. I can make it just add the engines by default. Less to document and for the user to have to figure out (unless it would rub people the wrong way).

@curious-attempt-bunny

Okay. This pull request is now much simplified. What do you think? @shesek @visionmedia

@shesek

Looks fine to me. I think adding it by default makes sense, it shouldn't break anything to anyone.

@curious-attempt-bunny

I'd still like to see this make it's way into master. Currently I have express locally patched. Anything left to improve @visionmedia ?

@tj
tj commented

I dont like the idea of maintaining the npm version separately, maybe we can at least ditch that, my npm is at 1.2.x already I dont want to check up on that all the time

@jonathanong

this is too strict and would stop people from upgrading to newer versions of node and npm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 16, 2013
  1. @curious-attempt-bunny

    bin/express now generate engine entries for node and npm in package.json

    Merlyn Albery-Speyer authored curious-attempt-bunny committed
This page is out of date. Refresh to see the latest.
Showing with 3 additions and 0 deletions.
  1. +3 −0  bin/express
View
3  bin/express
@@ -348,6 +348,9 @@ function createApplicationAt(path) {
, scripts: { start: 'node app' }
, dependencies: {
express: version
+ }, engines: {
+ node: process.versions.node.replace(/\.\d+$/, '.x')
+ , npm: '1.1.x'
}
}
Something went wrong with that request. Please try again.