alexbrazier / simple-update-notifier

Simple update notifier to check for npm updates for cli applications
MIT License
17 stars 9 forks source link

Adds support for vendor prefixed packages #12

Closed voxpelli closed 1 year ago

voxpelli commented 1 year ago

When trying to add this to @socketsecurity/cli it didn't work and I spent some time trying to decipher the generated TS code and finally found the error:

It wanted to save the JSON-file at ~/.config/simple-update-notifier/@socketsecurity/cli.json and since the ~/.config/simple-update-notifier/@socketsecurity/ folder didn't exist it failed.

Sidenote

Since it doesn't report any of the failures I spent quite a while wondering why nothing was being printed in my CLI, as the above error was never printed: https://github.com/alexbrazier/simple-update-notifier/blob/a35efd0ab9ad9f6e300f16d1546c02d72d2a4e31/src/index.ts#L23-L25

And the published version of that simple function is this monstrous thing:

var simpleUpdateNotifier = function (args) { return __awaiter(void 0, void 0, void 0, function () {
    var latestVersion;
    return __generator(this, function (_b) {
        switch (_b.label) {
            case 0:
                if (!args.alwaysRun &&
                    (!process.stdout.isTTY || (isNpmOrYarn && !args.shouldNotifyInNpmScript))) {
                    return [2 /*return*/];
                }
                _b.label = 1;
            case 1:
                _b.trys.push([1, 3, , 4]);
                return [4 /*yield*/, hasNewVersion(args)];
            case 2:
                latestVersion = _b.sent();
                if (latestVersion) {
                    console.log(borderedText("New version of ".concat(args.pkg.name, " available!\nCurrent Version: ").concat(args.pkg.version, "\nLatest Version: ").concat(latestVersion)));
                }
                return [3 /*break*/, 4];
            case 3:
                _b.sent();
                return [3 /*break*/, 4];
            case 4: return [2 /*return*/];
        }
    });
}); };

Back to the solution

This PR adds a test for vendor prefixed names and makes that test pass by applying the conversion of the package name as @types/ packages does, in other words: Remove the @ and replace the / with __.

alexbrazier commented 1 year ago

Published as 1.0.8

voxpelli commented 1 year ago

@alexbrazier Yay! Thanks for quick merge and release, splendid!