Skip to content

feat: add custom npm registry when installing dependencies#994

Merged
antongolub merged 2 commits intogoogle:mainfrom
easymikey:provide-custom-npm-registry
Dec 16, 2024
Merged

feat: add custom npm registry when installing dependencies#994
antongolub merged 2 commits intogoogle:mainfrom
easymikey:provide-custom-npm-registry

Conversation

@easymikey
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
const deps = parseDeps(await fs.readFile(filepath))
await installDeps(deps, dir)
}

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ok

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 merged commit fb436fa into google:main Dec 16, 2024
@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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: provide npm registry customization

2 participants