Skip to content

fix: read npm registry from npm config #3882

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 11, 2018
Merged

fix: read npm registry from npm config #3882

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/node-package-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ export class NodePackageManager implements INodePackageManager {
}

public async getRegistryPackageData(packageName: string): Promise<any> {
const url = `https://registry.npmjs.org/${packageName}`;
const registry = await this.$childProcess.exec(`npm config get registry`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about falling back to registry.npmjs.org in case npm config get registry fails for some reason?
For example:

let registry = "https://registry.npmjs.org/";
try {
    registry = await this.$childProcess.exec(`npm config get registry`);
} catch (err) {
    this.$logger.trace("Unable to get information for current npm registry", err);
}

const url = registry.trim() + packageName;

Copy link
Author

@lambourn lambourn Sep 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this as well - defensive programming FTW ;-) But even if I wipe my .npmrc file, the value npm defaults to is always https://registry.npmjs.org/

Plus, if npm config get registry fails, then something is foul with npm installation in general.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks for the explanation. We'll run our tests now and if the build is green, I'll merge this PR.

const url = registry.trim() + packageName;
this.$logger.trace(`Trying to get data from npm registry for package ${packageName}, url is: ${url}`);
const responseData = (await this.$httpClient.httpRequest(url)).body;
this.$logger.trace(`Successfully received data from npm registry for package ${packageName}. Response data is: ${responseData}`);
Expand Down