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

[BUG] Adding modules from GitHub not working #7898

Open
2 tasks done
cmahnke opened this issue Nov 7, 2024 · 8 comments
Open
2 tasks done

[BUG] Adding modules from GitHub not working #7898

cmahnke opened this issue Nov 7, 2024 · 8 comments
Labels
Bug thing that needs fixing Needs Triage needs review for next steps

Comments

@cmahnke
Copy link

cmahnke commented Nov 7, 2024

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

When installing a depedency from Github with a given commit / revision hash, the required code should be installed to node_modules.
Example:

npm i --verbose  github:ProjectMirador/mirador#5cb692ed

Only packages.json and text files (LICENSE, README.md) are installed.

According to the documentation this is a valid notation, but others aren't working either...

Using Yarn this works and demonstrates that this is a serious bug:

yarn add  github:ProjectMirador/mirador#5cb692ed
yarn -v
1.22.22

Expected Behavior

npm should download and install the required dependency not just metadata, just as yarn does

Steps To Reproduce

  1. mkdir test
  2. cd test
  3. npm i --verbose github:ProjectMirador/mirador#5cb692ed
  4. ls node_modules/mirador/
    Sources are missing

Environment

  • npm: 10.9.0
  • Node.js: v22.9.0
  • OS Name: MacOS 15.0.1
  • System Model Name: MacBook Pro
  • npm config:
; node bin location = /Users/cmahnke/.nvm/versions/node/v22.9.0/bin/node
; node version = v22.9.0
; npm local prefix = /Users/cmahnke/projects/projektemacher/test
; npm version = 10.9.0
; cwd = /Users/cmahnke/projects/projektemacher/test
; HOME = /Users/cmahnke
; Run `npm config ls -l` to show all defaults.
@cmahnke cmahnke added Bug thing that needs fixing Needs Triage needs review for next steps labels Nov 7, 2024
@kchindam-infy
Copy link

@cmahnke Could you provide detailed, step by step instruction on how to reproduce the issue?, This should include all the commands run, starting from an empty directory, and any prerequisites steps you took before running the npm install.

@cmahnke
Copy link
Author

cmahnke commented Nov 15, 2024

@kchindam-infy

cd /tmp
mkdir test
cd test
npm i  github:ProjectMirador/mirador#5cb692ed

Afterwards:

ls node_modules/mirador/
LICENSE		README.md	package.json

Sources are missing

Context:

npm -v
10.9.0
node -v
v22.9.0

@kchindam-infy
Copy link

I have tried with the steps provided and i am unable to repro the issue as mentioned. Its erroring out. Appreciate if you provide more detailed steps on how to repro the issue.
2024-11-15T08_20_44_777Z-debug-0.log

@cmahnke
Copy link
Author

cmahnke commented Nov 15, 2024

@kchindam-infy: Thanks! That actually a good hint, might it be the case that it's failing silently (or just by setting a return code)? If yes, that should be considered a bug in itself of cause.

But I'll have a deeper look on my side first:
Installing creates two log files, this might caused by some lifecycle script?

Update

While creating a demonstration I could reproduce your findings, they are related to Automattic/node-canvas#2448
While I'll try to figure out if this module can be deprecated upstream I still try to figure out if this is the root cause for my report, since this error actually is reported...

@cmahnke
Copy link
Author

cmahnke commented Nov 15, 2024

@kchindam-infy
I've created a reproducable example here: https://github.com/cmahnke/npm-7898
It provides the required libraries to build canvas and shows, that the error you've reported is not the root cause.
Hopefully this can help you, I can also setup a GitHub action...

@kchindam-infy
Copy link

@cmahnke I have reviewed the package.json of ProjectMirador
The files Field in package.json
The package.json of mirador specifies:

"files": [
"dist"
],
This means that only the dist directory is included when the package is published to npm or installed via npm from GitHub.

  1. The .gitignore Excludes the dist Directory
    The .gitignore file includes:
    dist/
    Therefore, the dist directory is not present in the GitHub repository.

  2. Installing from GitHub via npm
    When you install a package directly from GitHub using npm (e.g., npm install github:user/repo), npm fetches the repository content as is.
    Since the dist directory is not in the repository and is the only directory included via the files field, the installed package ends up being empty except for files like package.json, LICENSE, and README.md.

That's why you're only seeing minimal files in node_modules/mirador/.
The built code (in dist/) is not included in the GitHub repository, so it's not available when installing directly from GitHub via npm.

@cmahnke
Copy link
Author

cmahnke commented Nov 25, 2024

@kchindam-infy Thanks for the clarification, so basically it's an error that this works in yarn?

I'm afraid there is no workaround?

@kchindam-infy
Copy link

As an workaround, you can manually trigger the build process of mirador after installing it with npm by adding it as a dependency in your package.json and including a postinstall script. Specifically, add "mirador": "github:ProjectMirador/mirador#5cb692ed" to your "dependencies" and include a "postinstall" script like "postinstall": "cd node_modules/mirador && npm install && npm run build". This script will navigate into the mirador directory after installation, install its own dependencies, and run the build command to generate the necessary files. By doing this, when you run npm install, it will install mirador from GitHub and automatically build it, ensuring all required files are present in node_modules/mirador/. This workaround allows you to continue using npm without switching to yarn.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Needs Triage needs review for next steps
Projects
None yet
Development

No branches or pull requests

2 participants