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

feat: add custom npm registry when installing dependencies #994

Merged
merged 2 commits into from
Dec 16, 2024

Conversation

easymikey
Copy link
Contributor

@easymikey easymikey commented Dec 16, 2024

Description

Add custom npm registry with composition install and registry flags.

Fixes #972

zx --install --registry='https://npm-proxy.example.com' script.mjs
Screenshot 2024-12-16 at 15 41 17
  • Tests pass
  • Appropriate changes to README are included in PR

@easymikey
Copy link
Contributor Author

@antongolub you can start the review:)

@easymikey easymikey force-pushed the provide-custom-npm-registry branch from fc665c3 to e9d53d2 Compare December 16, 2024 12:53
src/cli.ts Outdated
--eval=<js>, -e evaluate script
--ext=<.mjs> default extension
--install, -i install dependencies
--install-registry<path> install install dependencies via custom npm registry URL
Copy link
Collaborator

Choose a reason for hiding this comment

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

--install-registry= install registry URL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

d1

src/cli.ts Outdated
@@ -208,6 +217,12 @@ export async function importPath(
const deps = parseDeps(await fs.readFile(filepath))
await installDeps(deps, dir)
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's just modify the current flow:

if (argv.install) {
    const deps = parseDeps(await fs.readFile(filepath))
    await installDeps(deps, dir, argv['install-registry'])
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you suggesting to make such a flow? Maybe then replace --install-registry with --registry as in npm?

zx --install --registry='https://npm-proxy.example.com' script.mjs

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok

Copy link
Contributor Author

Choose a reason for hiding this comment

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

d1

@easymikey easymikey force-pushed the provide-custom-npm-registry branch from cb1df6c to a13f2c2 Compare December 16, 2024 14:49
@easymikey easymikey requested a review from antongolub December 16, 2024 14:56
src/cli.ts Outdated
--eval=<js>, -e evaluate script
--ext=<.mjs> default extension
--install, -i install dependencies
--registry=<npm registry URL> custom npm registry URL dependencies works with install flag
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd suggest:
--registry=<URL> npm registry, defaults to https://registry.npmjs.org/

Update also man/zx.1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool suggest!

I update man/zx.1 and description for cli

Copy link
Collaborator

@antongolub antongolub left a comment

Choose a reason for hiding this comment

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

One more suggestion.

src/cli.ts Outdated
--help, -h print help
--repl start repl
--experimental enables experimental features (deprecated)
--quiet suppress any outputs
Copy link
Collaborator

Choose a reason for hiding this comment

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

Now we can keep the formatting here for git blame.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks to your suggestion. Revert formatting

@easymikey easymikey force-pushed the provide-custom-npm-registry branch from cc3fca1 to 0aca5e8 Compare December 16, 2024 18:25
Copy link
Collaborator

@antongolub antongolub left a comment

Choose a reason for hiding this comment

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

Seems fine. Thanks for the improvement!

@antongolub antongolub added the ossln24 OSS Library Night 2024 label Dec 16, 2024
@antongolub antongolub merged commit fb436fa into google:main Dec 16, 2024
21 checks passed
@easymikey easymikey deleted the provide-custom-npm-registry branch December 17, 2024 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ossln24 OSS Library Night 2024
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: provide npm registry customization
2 participants