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

preferLocal option does not run dependencies of dependencies #169

Closed
AndyOGo opened this issue Jan 16, 2019 · 14 comments
Closed

preferLocal option does not run dependencies of dependencies #169

AndyOGo opened this issue Jan 16, 2019 · 14 comments

Comments

@AndyOGo
Copy link

AndyOGo commented Jan 16, 2019

Dependencies of locally installed packages aren't found and thus cause ENOENT errors.

May I guess local node modules aren't resolved like require.resolve?

It throws:

Error: spawn npmversion ENOENT

A workaround is to add the needed package as peerDependencies, so that the package is directly available at the consumer, like is done here https://github.com/axa-ch/generic-release/pull/24/files

@AndyOGo AndyOGo changed the title dependencies of local npm packages are not found dependencies of local npm packages are not found ENOENT Jan 16, 2019
@ehmicky
Copy link
Collaborator

ehmicky commented Mar 6, 2019

@AndyOGo could you please provide steps to reproduce the issue?

@ehmicky
Copy link
Collaborator

ehmicky commented Apr 30, 2019

I think I now understand your problem, let me rephrase it: the preferLocal option does not allow running nested dependencies (dependencies of dependencies).

I don't think this is an issue. The way npm works (and most Node libraries) requires explicit declaration of dependencies to use them (either programatically or through the command line). This also seems like a good thing, as you are being explicit about a CLI that your program is using.

peerDependencies is not a workaround, it's the actual solution for this kind of problem.

@sindresorhus can we close this issue?

@ehmicky ehmicky changed the title dependencies of local npm packages are not found ENOENT preferLocal option does not run dependencies of dependencies Apr 30, 2019
@AndyOGo
Copy link
Author

AndyOGo commented Apr 30, 2019

@ehmicky
@sindresorhus
Thanks for your answer.

Right, it's about nested dependencies.

To be clear we release a node package called @axa-ch/generic-release which has npmversion as it's own dependency. And this dependency is not found if our binary is executed in the scope of another project.

So it's about the dependencies of the dependent node package.
execa should be able to resolve these properly, please reopen this issue.

Node provides the perfect API for this case already by require.resolve.

Please let me make the point clear, that require.resolve is the proper solution, not peerDependencies.

@ehmicky
Copy link
Collaborator

ehmicky commented Apr 30, 2019

Ok I think I see what you're trying to do with @axa-ch/generic-release. I actually have my release logic abstracted to its own module as well, so I understand where you're coming from.

Now I must be missing something because node_modules/.bin/ (which is where preferLocal will find your binary) would contain the npmversion binary under your setup, i.e. everything should work. You should not even need require.resolve().

For example, let's say:

  • you have a package called @axa-ch/example
  • @axa-ch/example has a devDependency called @axa-ch/generic-release
  • @axa-ch/generic-release has a production dependency called npmversion
  • both @axa-ch/generic-release and npmversion have binaries (bin field in package.json)

Under that setup, when @axa-ch/example call npm install, both binaries should be available under node_modules/.bin/, i.e. executable with execa. This is because npm automatically fills the top-level node_modules/.bin/ directory with the binaries of all nested dependencies.

Please let me know what I'm missing here.

Note: shouldn't you call npmversion programatically instead of through the CLI? It seems easy to do looking at their code. It would be faster as you would avoid spawning a child process. It would also solve any issues with nested dependency loading.

Note 2: you might consider other projects like release-it which are much more maintained and with more features. But that's just a tip you can discard. Unrelated to this issue.

@AndyOGo
Copy link
Author

AndyOGo commented Apr 30, 2019

@ehmicky Thanks for your quick answer.

I agree, and I also thought that node_modules/.bin would be resolved of any nested deps, cause npm creates a flat directory structure. Unfortunately turned out my understanding of this was wrong.

We use execa because we needed a cross-platform way to automatically create SEMVER valid versions, run some git, npm and npmversion commands automatically based upon user input.

Indeed, you are right, would also be possible to call npmversion programatically.

Nevertheless this is an issue for all nested npm packages not being resolved by execa.

@ehmicky
Copy link
Collaborator

ehmicky commented Apr 30, 2019

I agree, and I also thought that node_modules/.bin would be resolved of any nested deps, cause npm creates a flat directory structure. Unfortunately turned out my understanding of this was wrong.

Could you share your setup? I am still not understanding how you're having an issue there.

I assume the following assumptions to be true:

  1. all nested npm packages binaries should be available in node_modules/.bin/
  2. as a consequence, execa should find them and execute them.

Which of the assumptions are wrong? If one of them is wrong, could please provide your setup?

FYI execa simply adds node_modules/.bin/ to the $PATH, so I'm not sure how assumption 2) could be wrong.

@AndyOGo
Copy link
Author

AndyOGo commented Apr 30, 2019

I quickly revised my understanding.

As per NPM docs seems that only locally installed dependencies are available for npm run scrips:

In addition to the shell’s pre-existing PATH, npm run adds node_modules/.bin to the PATH provided to scripts. Any binaries provided by locally-installed dependencies can be used without the node_modules/.bin prefix. For example, if there is a devDependency on tap in your package, you should write:

"scripts": {"test": "tap test/\*.js"}

Here is one of our public setups:

  1. @axa-ch/patterns-library (a collection of web-components) has @axa-ch/generic-release in it's devDependencies:
    https://github.com/axa-ch/patterns-library/blob/c2ecb5a186903aa607291794cb78495a86737ebc/package.json#L83
  2. and use it's binary within the npm scrips for releasing:
    https://github.com/axa-ch/patterns-library/blob/c2ecb5a186903aa607291794cb78495a86737ebc/package.json#L44
  3. @axa-ch/generic-release has execa and npmversion within it's dependencies:
    https://github.com/axa-ch/generic-release/blob/8e15bfde4632353ba9a4b8786dcae18f9df7fd5e/package.json#L18-L19

The issue is that executing @axa-ch/generic-release within the scope of @axa-ch/patterns-library execa doesn't find npmversion if it's not locally installed in that scope - so I added it as peerDependencies too:
https://github.com/axa-ch/generic-release/blob/8e15bfde4632353ba9a4b8786dcae18f9df7fd5e/package.json#L23-L25

@ehmicky
Copy link
Collaborator

ehmicky commented Apr 30, 2019

npm docs says that locally-installed dependencies can be used without the node_modules/bin/ prefix but it does not say that nested dependencies cannot be used. As a matter of fact they can. If you check npm source code, they basically do the same as execa (add node_modules/.bin/ to $PATH).

The setup you provided above does not produce any issues for me. Reproducible steps:

$ git clone https://github.com/axa-ch/patterns-library.git
$ cd patterns-library
$ npm install
$ vim node_modules/@axa-ch/generic-release/lib-release.js
$ npm run release
> @axa-ch/patterns-library@2.0.1-beta.264 release /home/ehmicky/patterns-library
> generic-release docs dist

Node-Version 1.7.0

A command line node module to deal with "bumping" and "npm version"
It should be execute into a folder with a package.json file

Usage: version [options]

Options:
    --help
        Print the help around the command

    -i --increment [<level>]
        Increment a version by the specified level.  Level can
        be one of: major, minor, patch, premajor, preminor,
        prepatch, or prerelease.  Default level is 'patch'.
        Only one version may be specified. A Git commit and
        tag will be created.
        Nota Bene: it will use the "npm version" command if the option
        "read-only" is not activated.

        -p --preid <identifier>
            Identifier to be used to prefix premajor, preminor,
            prepatch or prerelease version increments. It could
            be 'snapshot', 'beta' or 'alpha' for example.

        --force-preid
            If specified, we force to add if needed the specified preid

        --read-only
                Print only the future version. Don't modify the package.json file,
                nor the npm-shrinkwrap.json file, don't create a commit and don't
                create a git tag

        --nogit-commit
            No git commit

        --nogit-tag
            No git tag

        --git-create-branch
            Create a new branch. Does not work if a preid is generated.

        --git-push
            Push the commit, the branch and the tags if needed

    -u  --unpreid
        Remove the prefix. The increment and preid option will be ignored.
        Only a Git commit will be created

        --read-only
               Print only the future version. Don't modify the package.json file,
               nor the npm-shrinkwrap.json file, don't create a commit and don't
               create a git tag

       --nogit-commit
           No git commit

       --nogit-tag
           No git tag

        --git-create-branch
            Create a new branch. Does not work if a preid is generated.

        --git-push
            Push the commit, the branch and the tags if needed


🚀  Hello Dear developer, welcome to the release assistant. 🚀

!! Please make sure you have no changes to be commited !!

I'm getting some information....

>>> resolved | npm whoami
>>> resolved | npm owner ls
Attention: Your account ehmicky has no publisher rights. Please contact the administrator

In the vim command, I added the following line:

#!/usr/bin/env node

const readline = require('readline');
const outdent = require('outdent');
const execa = require('execa');
+ console.log(execa.sync('npmversion').stdout)

I.e. execa does find npmversion within generic-release.

I don't think there is an issue with execa there. If you can get a reproducible setup which produces the bug, we can look into it.

@AndyOGo
Copy link
Author

AndyOGo commented Apr 30, 2019

@ehmicky

Thank you so much for looking into this and investing your valuable time.

Please watch out, as I said I declared npmversion as peerDependencies.
So I also added npmversion to the devDependencies of @axa-ch/patterns-library, as can be seen here:
https://github.com/axa-ch/patterns-library/blob/develop/package.json#L121

Without that it would not work.

@ehmicky
Copy link
Collaborator

ehmicky commented Apr 30, 2019

If you remove npmversion from the devDependencies of @axa-ch/patterns-library (and from the peerDependencies of @axa-ch/generic-release), the bug still does not appear. I just checked it.

npmversion is still installed in node_modules/.bin/, and execa still manages to find it.

@AndyOGo
Copy link
Author

AndyOGo commented Apr 30, 2019

Interesting, I observe other behaviour.

Which OS are you using? We need to support Windows, Linux and macOS

I received this ENOENT error:

proceed: to proceed with the above described steps. This operation cannot be undone!
proceed
>>> resolved | git checkout develop --quiet
>>> resolved | git pull
>>> resolved | git checkout -b release-tmp --quiet
Step 1 complete...
>>> resolved | npm run test
>>> rejected | npmversion --increment prerelease --preid beta --force-preid --nogit-commit --nogit-tag
Error: spawn npmversion ENOENT

@ehmicky
Copy link
Collaborator

ehmicky commented Apr 30, 2019

I am on Linux. OS difference might be a thing, because npm install things quite differently on Windows. Are you on Windows?

@AndyOGo
Copy link
Author

AndyOGo commented Apr 30, 2019

I'm on macOS, most people are on Windows

@ehmicky
Copy link
Collaborator

ehmicky commented Apr 30, 2019

Alright if you're on macOS, there should be no difference in how npm installs things.

I think the next step at this stage would be to get a minimally reproducible example. The setup above is quite complex and could be reduced to smaller parts.

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

3 participants