[rush] "rush add" should be able to add unpublished locally-linked projects

[cortopy]

I come from the lerna world, and I can't get my head round the fact that you can't add -p <package> when that package is internal and has not been published yet.

When you do lerna add it will first check the npm registry, and if none available it will check if it's one of the packages in the monorepo.

For now, I'm adding a line in the package.json of each file. But this forces me to do rush update -p --full each time, which is cumbersome.

How does one link dependencies within the monorepo itself with Rush? If there's a way I'd love to know. If not, I'd love something like lerna even more

The only information I could find about linking packages is this issue and the solution does not seem to be supported in rushjs 5.

[octogonz]

I come from the lerna world, and I can't get my head round the fact that you can't add -p <package> when that package is internal and has not been published yet.

Hmmm... We don't use Lerna regularly, so when we designed the rush add feature we hadn't thought about this case at all. But it seems obvious enough that we decided to tag it as a "bug" instead of an "enhancement". :-)

For now, I'm adding a line in the package.json of each file. But this forces me to do rush update -p --full each time, which is cumbersome.

Are you sure? The --purge option generally should be needed only if your common/temp folder got corrupted somehow. And the --full option is generally needed only if you're intentionally wanting to do a global update. rush update should work in this situation and should be pretty fast.

Which package manager are you using with Rush?

How does one link dependencies within the monorepo itself with Rush?

It happens automatically. If a project registered in rush.json satisfies the SemVer range for a dependency, it will get locally linked. The only exception is when cyclicDependencyProjects is used. (See issue #547 for some additional thoughts about that.)

[octogonz]

@nickpape-msft FYI

[cortopy]

thanks @pgonzal for looking into this!!

Are you sure? The --purge option generally should be needed only if your common/temp folder got corrupted somehow. And the --full option is generally needed only if you're intentionally wanting to do a global update. rush update should work in this situation and should be pretty fast.

I was getting a lot of Unhandled rejection Error: Integrity check failed: errors when not running with those options. I'm not sure what I was doing wrong, as I'm still trying to understand what each command and option does

Which package manager are you using with Rush?

npm v6.5.0

[octogonz]

I was getting a lot of Unhandled rejection Error: Integrity check failed: errors when not running with those options. I'm not sure what I was doing wrong, as I'm still trying to understand what each command and option does

Which package manager are you using with Rush?

npm v6.5.0

Sounds like the same issue as #886 . We do need to get this fixed. But it seems like every time a person comes along who's excited to pitch in and improve the NPM support, they end up switching to PNPM or Yarn. Even the NPM developer seemed to lose interest in fixing the regression discussed in that thread.

[cortopy]

In my case I started with PNPM but it was giving me so many issues I decided to stick with what I know (either npm or yarn. I read yarn support is still experimental so picked npm). I was hitting my limit of fixing issues related to new tooling and I rather focus on one thing (i.e.: rush)

[octogonz]

@WanderWang encountered this in a Gitter chat today.

[halfnibble]

We tried to repro this today using rush add -p <local-package> and it just worked. We used pnpm version 3. Also note, the local package was listed in the rush.json config (which I don't think is an unreasonable assumption). @WanderWang can you link us to your repo or can someone send us a minimal repro?

[GongT]

https://github.com/GongT/issues/tree/web-build-tools-991

[VincentSurelle]

So I've been through the problem today. I what I thought to be one problem is actually two.

First, I was trying to install a dependency with a local projet

rush add -p @swsjs/gulp-tools and it didn't worked because of this : #1469 (comment)

Then I changed my package name from @swsjs/gulp-tools to gulp-tools

rush add -p gulp-tools worked, and linked local version.

But !

This was because if you look at https://npmjs.com, the package gulp-tools already exists.

Since I'm now trying to rush add -p common-lib-node and it doesn't exists on npm registry, it doesn't work.

[VincentSurelle]

I'd like to make a PR for this since it's blocking for us. But I prefer to talk about design in there before making it ;)

The thing we're trying to solve here is to have unpublished projects locally linked.
Actually, rush require a valid package name. So this design reject the possibility to have local filesystem projects installed directly (like : https://pnpm.js.org/en/cli/add#install-from-local-file-system)

Allowing it, may be reasonable for projects that are made inside the monorepo.

If you breaking-change your lib, every projects using it inside the monorepo will have to change to work. But that's actually how it works.

You don't really need versions. Especially if your lib is only used inside the monorepo and never gets published.

So if I rush add -p <path-present-in-rush-json> the package.json file will look like :

"dependencies": {
  "my-lib": "link: src/my-lib"
}

I may be too new in the rush code to see side effects for now so, what do you think about this idea ?

[alex-shamshurin]

Any update on this? I spent a lot of time thinking how to add local package (which not intended to be published) to package.json via "rush add". The only way I found is adding manually to package.json with version and this looks weird.

[halfnibble]

@VincentSurelle Thanks for the helpful info!

In my opinion, one should be able to run rush add -p <local_package> and if the package is not published on npm, Rush should automatically create a symlink entry with a path based on the rush.json config. If the local package is not in the rush.json config, then an error should be generated.

@alex-shamshurin You can do this manually to mitigate, but I agree it is awkward. @Claudiazhaoya is currently working on this issue.

[VincentSurelle]

@halfnibble Ok understood ;)

So there are 3 use cases with local packages (with questions) :

I'm using a package my-lib that is not published :
Rush will fail to find it on npm, so it will look on rush.json. If found it will be linked.

I'm using a package my-lib that is published :
This way, Rush will find it on npm and also in local. So local package will be linked ?

I'm using a package my-lib that is published but by someone else (common name) :
If I use a package name that already exists on npm but it's not mine, will it still look for the local package ?

I think we should document well that the package must not have a generic name. Maybe @scope it even if it's not published on npm to make sure there are no collisions

[halfnibble]

@VincentSurelle Rush currently handles the latter two cases in a way that I feel is appropriate. If a published package is found, and there is a package with the same name in rush.json, then Rush will symlink the local package, but only if the version in the local package's package.json meets the requested version. Otherwise, Rush will download the package from npm.

The third case is kind of an edge case. It is possible that someone wants to try their own modified variant of someone else's package locally. Or it could be a naming collision by mistake. Perhaps a warning should be generated, but right now, the rules stated above will apply. I like the idea of mentioning the potential for naming collisions in the docs.

The only case that is not handled at all is the first case.

[Jabher]

As long as rush is forcing to use approach "modify first, bump version second" the case when published and local versions match, but code differs is expected on my point of view. Isn't it?

Also, I think non-working case 1 is show-stopper in lots of cases

[iclanton]

This has been fixed in 5.17.1