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

Improve --install #525

Open
zloirock opened this issue Oct 8, 2022 · 13 comments
Open

Improve --install #525

zloirock opened this issue Oct 8, 2022 · 13 comments

Comments

@zloirock
Copy link
Contributor

zloirock commented Oct 8, 2022

--install option is an interesting feature. When you maintain a big project, it could significantly help you - for example, avoid loading to CI unnecessary heavy dependencies that are used only in some scripts. However, now this option has some significant issues.

  • At this moment, this option install all dependencies of the script - even if this dependency is already installed at the root of the project.
  • It's problematic to control versions of dependencies if they are defined as comments of imports - this dependency can be used in some different scripts, the actual tools for updates checking can't get access to versions from those comments.
  • Dependencies from the monorepo (workspaces) also installed and resolved incorrectly.
  • It's problematic to configure installing process.

  • The first issue can be solved by checking the existence of the dependency in the top-level node_modules of the project before installing.
  • The second can be solved by adding a way to define those versions in a package.json (top-level of the project or defined in another place) without installing those dependencies. In the case of the top-level package.json, it can be optionalDependencies.
@antongolub
Copy link
Collaborator

antongolub commented Oct 10, 2022

At this moment, this option install all dependencies of the script - even if this dependency is already installed at the root of the project.

We can avoid this side effect by using the package manager: npx -p dep1@v1 -p dep2@* -p depN@^3.0.1 -p [email protected] zx script.js

@zloirock
Copy link
Contributor Author

We can avoid this side effect by using the package manager: npx -p dep1@v1 -p dep2@* -p depN@^3.0.1 -p [email protected] zx script.js

However, this way is not so convenient as just zx -i and controlling dependencies versions turns to hell.

@zloirock
Copy link
Contributor Author

zloirock commented Oct 10, 2022

Today I played with it and wrote a simple zxi wrapper for zx and install helper, 2 additional commits show how they can be used.

They get required versions of packages from lazyDependencies field of package.json, if the required version of the package is already present - it will not be installed, if all of them - npm i will not be called at all. Sure, it works only with the root package.json and node_modules, for zx it could be good also install dependencies of local dependencies - but it can be simply extended for that.

It could be good to have similar functionality just with zx -i.

@antonmedv
Copy link
Collaborator

At this moment, this option install all dependencies of the script - even if this dependency is already installed at the root of the project.

But is it a problem? Will npm check what package is already installed and it will be noop.

It's problematic to control versions of dependencies if they are defined as comments of imports - this dependency can be used in some different scripts, the actual tools for updates checking can't get access to versions from those comments.

I this this is a feature! Each script can have each own versions. One is using ^2, other ^1. Yes ncu will not detect version/packages. For outdated versions we can create separate package for checking and updating comments (zx-check-updates).

Dependencies from the monorepo (workspaces) also installed and resolved incorrectly.

I'm not familiar with this setup. ¯\_(ツ)_/¯

It's problematic to configure installing process.

For example?

The second can be solved by adding a way to define those versions in a package.json (top-level of the project or defined in another place) without installing those dependencies. In the case of the top-level package.json, it can be optionalDependencies.

What about different version per script? Also Keeping deps and version in one place is better.

@zloirock
Copy link
Contributor Author

zloirock commented Oct 10, 2022

But is it a problem? Will npm check what package is already installed and it will be noop.

In some cases, this check can take a long time - 2 days ago in one project it took about 5 minutes without installing anything - and it was one of the reasons why I wrote this issue.

I this this is a feature! Each script can have each own versions. One is using ^2, other ^1. Yes ncu will not detect version/packages. For outdated versions we can create separate package for checking and updating comments (zx-check-updates).

A good practice is defining dependencies in one place - each new place where dependencies are defined, even with a new tool for controlling versions - is a pain. It's a good practice to reuse one dependency in different places of the project - however, if required to use different versions, it's not a big problem to specify it as a comment / in a local, not the root, package.json.

I'm not familiar with this setup. ¯_(ツ)_/¯

However, it's a significant use case. For example, here we use core-js-compat from the monorepo, but zx -i will install it from NPM.

For example?

I think that already shown cases are enougth.

What about different version per script?

See above.

@antonmedv
Copy link
Collaborator

In some cases, this check can take a long time - 2 days ago in one project it took about 5 minutes without installing anything - and it was one of the reasons why I wrote this issue.

I guess we can check what node_modules/foo exists, but what about checking the version? Why npm is so slow about it?

A good practice is defining dependencies in one place - each new place where dependencies are defined, even with a new tool for controlling versions - is a pain. It's a good practice to reuse one dependency in different places of the project - however, if required to use different versions, it's not a big problem to specify it as a comment / in a local, not the root, package.json.

We already have package.json and zx scripts can use project deps. The --install feature is useful for standalone scripts, or scripts fetched from the web.

However, it's a significant use case.

Let's explore the problem space )

@zloirock
Copy link
Contributor Author

zloirock commented Oct 10, 2022

I guess we can check what node_modules/foo exists, but what about checking the version? Why npm is so slow about it?

At least, NPM uses ranges and checks for new versions.

We already have package.json and zx scripts can use project deps. The --install feature is useful for standalone scripts, or scripts fetched from the web.

Such scripts can be in the scope of the project - for example, for CI is useful granular loading of dependencies - in one task linting, in another NodeJS testing (with different versions, so cache does not work), in one more - browsers, etc. Some scripts can be standalone and used only locally - but too often they should reuse the same dependencies as the rest project.

Let's explore the problem space )

I already updated this comment:

For example, here we use core-js-compat from the monorepo, but zx -i will install it from NPM.

@zloirock
Copy link
Contributor Author

(Note that I forgot that npm i --no-save pkg from npm@7 unconditionally prunes dependencies and that makes this task a little harder - workspaces can be simpler.)

@antonmedv
Copy link
Collaborator

Let’s try to break down to a few smaller parts what easier to implement.

@zloirock
Copy link
Contributor Author

zloirock commented Nov 5, 2022

It can be related oven-sh/bun#1459

@zloirock
Copy link
Contributor Author

Is this completed? I don't see anything about it.

@antongolub
Copy link
Collaborator

antongolub commented Mar 29, 2024

I think zx should not try to replace the package manager. I suggest using script composition for complex corner cases instead:

  1. zx -e 'const deps = fs.readJsonSync("package.json") ...' && zx script.js
import {$, cd} from 'zx'

// read your deps from anywhere
const deps = Object.entries(fs.readJsonSync("package.json").dependencies).reduce(...).join(' ')

// need isolated scope?
// cd('your/temp')
// check `./node_modules`, verify ranges with semver, modify deps, etc

// use any package manager for setup
await $`npm i ${deps}`

// use zx to run zx :-)
await $`zx script.js`

@zloirock
Copy link
Contributor Author

@antonmedv it's not about a replacement of the package manager, it's about calling a package manager when it's required. Just like in this wrapper, but from the box - since for my use cases that's required too frequently.

@antonmedv antonmedv reopened this Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants