Skip to content

feat(@schematics/update): add full support of npmrc / npm config #12350

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

Closed
wants to merge 1 commit into from

Conversation

smnbbrv
Copy link
Contributor

@smnbbrv smnbbrv commented Sep 22, 2018

A logical consequence of #12284.

Fully fixes #10624 and related issues.

The current npm config support is not full and most likely cannot ever become one. There are lots of (legacy) authentication methods, hierarchical .npmrc, all that is running in various OS, etc.

This is a brand new approach: npm is used as a service. It appears that internally npm not only allows reading all the configuration (including _auth, _authToken etc), but also already provides a resolver for various authentication models.

What is changed:

  • reduced file size by a factor of 2
  • improved performance
  • less async, more sync => way more readable, and, btw, more performant

and the most important:

  • so much less dirty code
  • there is no need to deal with NPM config and authentication anymore: NPM does it as NPM would do it

How to try on existing @angular/cli project:

  • npm i -D npm
  • paste the code below into your_project/node_modules/@schematics/update/update/npm.js
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
const fs_1 = require("fs");
const npm = require("npm");
const rxjs_1 = require("rxjs");
const operators_1 = require("rxjs/operators");
const url = require("url");
const RegistryClient = require('npm-registry-client');
const npmPackageJsonCache = new Map();
const ensuredNpm = new rxjs_1.Observable(subject => {
    npm.load(() => {
        subject.next();
        subject.complete();
    });
}).pipe(operators_1.shareReplay());
function getNpmConfigOption(option, scope) {
    if (scope) {
        return npm.config.get(`${scope}:${option}`) || getNpmConfigOption(option);
    }
    return npm.config.get(option);
}
function getNpmClientSslOptions(strictSsl, cafile) {
    const sslOptions = {};
    if (strictSsl === 'false') {
        sslOptions.strict = false;
    }
    else if (strictSsl === 'true') {
        sslOptions.strict = true;
    }
    if (cafile) {
        sslOptions.ca = fs_1.readFileSync(cafile);
    }
    return sslOptions;
}
/**
 * Get the NPM repository's package.json for a package. This is p
 * @param {string} packageName The package name to fetch.
 * @param {string} registryUrl The NPM Registry URL to use.
 * @param {LoggerApi} logger A logger instance to log debug information.
 * @returns An observable that will put the package.json content.
 * @private
 */
function getNpmPackageJson(packageName, registryUrl, logger) {
    const scope = packageName.startsWith('@') ? packageName.split('/')[0] : undefined;
    return ensuredNpm.pipe(operators_1.map(() => {
        const partialUrl = registryUrl
            || getNpmConfigOption('registry', scope)
            || 'https://registry.npmjs.org/';
        const partial = url.parse(partialUrl);
        let fullUrl = new url.URL(`http://${partial.host}/${packageName.replace(/\//g, '%2F')}`);
        try {
            const registry = new url.URL(partialUrl);
            registry.pathname = (registry.pathname || '')
                .replace(/\/?$/, '/' + packageName.replace(/\//g, '%2F'));
            fullUrl = new url.URL(url.format(registry));
        }
        catch (_a) { }
        logger.debug(`Getting package.json from '${packageName}' (url: ${JSON.stringify(fullUrl)})...`);
        return fullUrl;
    }), operators_1.switchMap(fullUrl => {
        let maybeRequest = npmPackageJsonCache.get(fullUrl.toString());
        if (maybeRequest) {
            return maybeRequest;
        }
        const subject = new rxjs_1.ReplaySubject(1);
        const auth = npm.config.getCredentialsByURI(fullUrl.toString());
        const client = new RegistryClient({
            proxy: {
                http: getNpmConfigOption('proxy'),
                https: getNpmConfigOption('https-proxy'),
            },
            ssl: getNpmClientSslOptions(getNpmConfigOption('strict-ssl'), getNpmConfigOption('cafile')),
        });
        client.log.level = 'silent';
        client.get(fullUrl.toString(), { auth, timeout: 30000 }, (error, data) => {
            if (error) {
                subject.error(error);
            }
            subject.next(data);
            subject.complete();
        });
        maybeRequest = subject.asObservable();
        npmPackageJsonCache.set(fullUrl.toString(), maybeRequest);
        return maybeRequest;
    }));
}
exports.getNpmPackageJson = getNpmPackageJson;

@clydin
Copy link
Member

clydin commented Sep 23, 2018

Thank you for the effort on this contribution. However, depending directly on npm is not a viable solution. Beyond the license validation issues, the dependency tree is massive. But most importantly, there does not appear to be an official public API which means the functions being used cannot be relied upon to continue to exist nor function as currently assumed. This presents a serious ongoing maintenance concern.
Yarn support is also being considered as an addition to ng update and it would definitely not be viable to depend directly on yarn as well.

@smnbbrv
Copy link
Contributor Author

smnbbrv commented Sep 23, 2018

hi @clydin

thank you for quick review!

Let's have a look at those problems:

  • license validation: I don't really see any problem with their license; of course I'm not a lawyer but they allow everything you want
  • the dependency tree: well, for the tool such as angular/cli it is not a big problem, is it? The dependencies don't go to the browser
  • no official API: the version of npm is fixed to 6.4.1. If there is a newer version that adds breaking changes you could still rely on 6.4.1 because it would be available locally in @schematics/update regardless of what version other packages might use

Let's have a look at what we currently have: ng update pretends to behave like NPM. So either one needs to use NPM to have functionality "as is" or massively replicate all NPM's features regarding auth and configuration.

Or, using NPM as a solution until they break the API and @schematics/update would need a newer version of NPM.

This issue #10624 is open since ng update was introduced and it is a real pain. Many packages starting to say: hey, run ng update to update to the actual version, but what if people cannot?

@smnbbrv smnbbrv closed this Sep 26, 2018
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"ng update" doesn't support private repos such as FontAwesome 5 Pro
3 participants