Skip to content

Conversation

@gethari
Copy link

@gethari gethari commented Nov 19, 2024

Fixes #424

@changeset-bot
Copy link

changeset-bot bot commented Nov 19, 2024

🦋 Changeset detected

Latest commit: c2796d4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@changesets/action Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@gethari
Copy link
Author

gethari commented Nov 20, 2024

@Andarist, could you kindly review this when you have a moment? I’m relying on this change to get my actions working and would prefer to avoid forking the repository for such a small update. Thank you!

// check based on https://github.com/npm/cli/blob/8f8f71e4dd5ee66b3b17888faad5a7bf6c657eed/test/lib/adduser.js#L103-L105
return /^\s*\/\/registry\.npmjs\.org\/:[_-]authToken=/i.test(line);
});
const authLine = extractAuthTokenLine(userNpmrcContent);
Copy link
Member

Choose a reason for hiding this comment

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

You have mentioned that the previous version of the code goes in to the else branch and that creates a problem. I don't understand this. Even if it goes there... it will just append a different token to ur .npmrc file and that's about it. Your correct token should still be used when publishing to a custom artifactory

Choose a reason for hiding this comment

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

No @Andarist, by creating this extract function, the code will goes to this block.

if (authLine) {
  core.info(
    "Found existing auth token for the npm registry in the user .npmrc file"
  );
}

and we will not create the other .npmrc.

const line = npmrcContent.split("\n").find((line) => {
return /^\s*\/\/.*\/:(_auth|_authToken)=/i.test(line); // Match both _auth and _authToken
});
return line ? line.trim() : undefined;
Copy link

Choose a reason for hiding this comment

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

Suggested change
return line ? line.trim() : undefined;
return line?.trim();

Choose a reason for hiding this comment

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

This is nitpick

@mazipan-wego
Copy link

This is very important fix for people that is not using default npm registry.
It's a show stopper bug.

Please help to drive this pull request.

@mazipan-wego
Copy link

Duplicate w #362

@gethari
Copy link
Author

gethari commented Sep 18, 2025

@Andarist do you want to consider merging this , if I resolve the conflicts ?

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

Successfully merging this pull request may close these issues.

Publish in custom artifactory

4 participants