thecodingmachine / nodejs-installer

An installer package that let's you install NodeJS and NPM as a Composer dependency.
107 stars 28 forks source link

Add extra.mouf.npm.version support #36

Closed hanovruslan closed 6 years ago

hanovruslan commented 6 years ago

How about add extra.mouf.npm.version support ?

moufmouf commented 6 years ago

That would be a great idea!

I believe it is not too difficult to implement. Just after installing NodeJS, we could try to install NPM with:

npm install npm@{$npm_version}

I believe we should do this here in the code.

The hardest part is that we need to change the logic of whether to install node locally or not.

So far, we install NodeJS locally only if it is not available globally OR if the global version does not match the requested constraint. With a extra.mouf.npm.version, I believe we should install NodeJS locally only:

I don't have much time to work on this right now, but if you are interested in working on a PR, I'd gladly accept it!

hanovruslan commented 6 years ago

HI! Good news ) I'll work on this ASAP

Some questions: 1) What are the difference between

OR if the global version does not match the requested constraint
OR if the NPM global version does not match the requested constraint

2) I guess that this module expects phpunit has been installed in global composer path. Is it correct? if so why not to install it as require-dev dependency?

moufmouf commented 6 years ago
  1. What I mean is this:

Let's assume you have NodeJS installed globally (v8.9) and NPM (v4). Now, if in your composer.json you specify this:

{
    "extra": {
        "mouf": {
            "nodejs": {
                "version": "^8"
            },
            "npm": {
                "version: "^6"
            }
        }
    }
}

What should we do? NodeJS is installed globally in v8.9, so it matches the constraint. With the current code, we would node install NodeJS locally as it is already available globally.

But in my example, NPM does not match. It is available globally in v4, but we are requesting v6. I believe we should not update NPM globally (maybe it is used by another program that requires v4). So we should certainly install NPM locally. But I'm not sure we can install NPM locally without also installing NodeJS locally.

  1. Definitely yes! You can add phpunit as a dev dependency. I don't know why I did not do it.
hanovruslan commented 6 years ago

I've decided to start from simple task - add phpunit as explicit dev dependency

Please, see #37

hanovruslan commented 6 years ago

Hi! I've read carefully about nodejs and npm releases and came to a decision that there isn't necessity for explicit npm version at all. Because all we have to do in order to get proper npm version is to select proper version of nodejs.

Yes, I've tested my hypothesis

I didn't analyse all of the supported platforms (only debian) platforms but i think that this is not so critical for the main purpose of this package

Sometimes developers mangle packages those installed in project and package binaries might come to version mismatching but in this case we need just this

rm -rf vendor/some-vendor/package \
  && rm vendor/bin/{some-vendor-binaries} \
  && composer install 
hanovruslan commented 6 years ago

I think this issue can be closed