Open bryanforbes opened 6 years ago
@dzearing @qz2017 FYI
Today, rush link
happens before any build process. How would the link manager (which runs during rush link
) handle linking to a non-existant folder (because build has not yet run)?
Would it also create a empty, placeholder folder if one does not exist?
Would it also create a empty, placeholder folder if one does not exist?
Yes. In fact, I started working on the changes for this and the link manager will do exactly that as it stands today.
Another question on my mind - you probably need to change rush change
too. Today, it detects changes from within a project folder, with the assumption that each project folder corresponds to exactly one published package. If there are 2 packages @my/library
and @my/library-plus
getting published from a single project folder's package.json, the semantics of rush change
as it was originally designed completely breaks. Could you describe how rush change
will be modified?
@qz2017 might have more comments/questions on the impact this might have on version policy.
I hadn't thought about how rush change
would need to be modified. Any suggestions would be welcome.
To me, that would be the most confusing aspect and drawback of this proposal. A folder's package.json would have exactly either @my/library
or @my/library-plus
but not both. The fact that it isn't representative is potentially confusing... and raises other questions (e.g. would all publishing variants of a projectFolder ALWAYS share the same versioning?).
So.. how to modify rush change
also depends on what your intent or what you think the right design should be here. At this point, I don't even dare start to think about the impact on version policies :)
Just brainstorming since I've already applied myself to your scenario... Without changing rush, maybe you could also accomplish what you're trying to do by having
{
"packageName": "@my/library-src",
"projectFolder": "packages/library-src",
// "shouldPublish": false
},
{
"packageName": "@my/library",
"projectFolder": "packages/library",
},
{
"packageName": "@my/library-plus",
"projectFolder": "packages/library-plus",
},
The build step of @my/library-src
would then do what you would have done in creating a publish variant in the resulting two folders...
A specific use case that this feature (publishing from and linking to a build artifact directory) would support is deep imports without the build directory (like dist
/lib
/build
) in the path; namely, importing from package/directory/module
instead of package/dist/directory/module
or just a bare import like package
.
Deep imports remove the need for workarounds like tree shaking and also enable more flexible API design, so that dependencies can be grouped in submodules instead of having to all be re-exported by a main module.
Lerna currently partially supports this feature with the publish --contents
flag, and np also has a --contents
flag.
This would be really good to have :)
@benjamind asked about this again in the Gitter forum. Recently Rush introduced an experimental setting useWorkspaces=true
in rush.json, which lets PNPM create the symlinks for local projects. This will become the default in the next major release of Rush.
With that new development, I think we need for PNPM to support this feature. PNPM would do the job of creating local links into the dist
folder. And then Rush's role would simply reduce to running npm publish
in the subfolder, which should be a pretty easy change.
Could someone investigate whether PNPM supports this already, or has an open issue requesting it?
PNPM now supports relative path (#2959) as references to other packages inside the workspace. This works fine to solve the dist
folder problem.
The only issue is that the version
and publish
process in Rush has to get updated to handles this correctly. Right now, Rush does not understand the relative path syntax and during version
it replaces the relative path with the version of the package.
@manrueda has created PR #2399 which implements a variant of the publishFolder
from above.
@bryanforbes originally proposed that the same projectFolder
could be mapped to different packageName
/publishFolder
settings, like this:
{
...
"projects": [
{
"packageName": "@my/library",
"projectFolder": "packages/library",
"publishFolder": "packages/library/release"
},
{
"packageName": "@my/library-plus",
"projectFolder": "packages/library",
"publishFolder": "packages/library/release-plus"
},
But with this design, when rush build
is invoked, how do we build packages/library
? It appears to be specified twice, and it no longer has a unique packageName
that can be used by the --to
CLI parameter.
Design A seems to be conflating two things:
pnpm run build
) pnpm pack
, possibly with a different package name and/or a different NPM version).It makes sense to generalize the design so that a single "project" can pack multiple "artifacts", but it's confusing to model those artifacts as entries in a "projects"
list.
@cliffkoh suggested that we could simply create fake "Rush project" items to represent the additional artifacts.
{
"packageName": "@my/library-src",
"projectFolder": "packages/library-src",
// "shouldPublish": false
},
{
"packageName": "@my/library",
"projectFolder": "packages/library",
},
{
"packageName": "@my/library-plus",
"projectFolder": "packages/library-plus",
},
This is a correct design, but the handling of package.json dependencies seems clumsy to me.
For example, suppose we have 10 Rush projects with a dependency graph among themselves. And suppose we publish a regular
and plus
edition of them. Then we'd need 30 projects (10 -src
, 10 -regular
, and 10 -plus
) and we'd seemingly need to manage 3 copies of the dependency graph.
If I understand PR #2399 correctly, @manrueda is implementing "publishFolder"
as a postprocessing step, while maintaining the idea that each "Rush project" has a single "published artifact." (Let me know if I misunderstood.)
So Design C tackles these requirements:
@internal
methods are stripped out during publishing, even though they are required for the build.)pnpm install
to create a valid node_modules
in the publishFolder
prior to publishingSome questions about Design C:
pnpm install
? The PR note says the packages being published must be part of the workspace and have the dependencies installed, but I was unclear whether this is an app requirement, or a technical detail of pnpm publish
.pnpm run build
? Or during a lifecycle event such as prepare
? Or should Rush provide its own lifecycle hook for this?publishFolder
outputs? If not, then should we standardize a name like packages/my-library/publish
and then the setting would be a simple boolean "publishFolder": true
. The Rush Stack effort has been seeking to standardize the top-level folders for a project (config
, dist
, lib
, temp
, src
, etc) and discourage deviations from that standard. In a large monorepo with lots of teams, having a regularized folder layout makes it much easier for newcomers to understand how a project is set up.@manrueda if you're able to share a branch (or config excerpt) showing how you intended to use this feature yourself, that might help speed this along. Despite all these design questions, I feel like we can still get your PR merged pretty fast once we sort out these design details. I'll ping you on Zulip to see if we can address some of them offline.
In https://github.com/microsoft/rushstack/issues/1416, @arielshaqed described another scenario involving the concord tool, which would also be solved by this same issue.
The publishFolder
setting was released with Rush 5.37.0. But we do not yet have a solution for symlinking to the built folder locally within the workspace. With Rush's new useWorkspaces=true
installation model, this problem is now primarily a feature request for PNPM.
Someone should find (or create) a GitHub issue for PNPM that we can link to.
Only to be sure, currently this setting is only used in the publish step and the project root is still symlinked in node modules? I try to create an angular monorepo with multiple libs in rush. To be able to use the build cache. My idea was to create multiple angular workspaces which contains only one specific lib and add a build npm command to the angular workspace. The compiled lib is located in the workspace folder under the dist/<lib-name>
folder. In this folder is also the generated package.json
for the lib. So the symlink of the project in this case needs to be <projectRoot>/dist/<lib-name>
. I have set a publishFolder but the symlink still points to the project root folder.
Or is there an easier way to setup a multi lib angular monorepo with rush?
I don’t want to hijack this thread so if there is an easier way where is the right place to ask this question? I have already tried SO https://stackoverflow.com/questions/67799073/rush-setup-angular-monorepo-with-libs
Hi @gkTim, I have a very similar setup as you describe. What I am doing (while not perfect), it to have all the package.json
metadata from the generated package.json
(main
, fesm2015
, esm2015
, etc fileds) in the root package.json
with the paths changed to point to the dist
folder. This will make node load the filed from dist
even when the symlink still points to the root of the project. Also this only works well if you use libraries in Ivy mode to avoid ngcc that makes thing super hard to cache and keep in sync.
@manrueda thanks for the hint. Was hoping for a less hacky solution. But angular libs don’t follow node best practices here. Maybe we can automate this steps.
@octogonz if this is done via pnmp workspaces this feature would not be supported by npm or yarn, right?
Am I right that the symlinks are done by the LinkManager on rush side or it’s specific implementation e.g. PnpmLinkManager?
If that’s the case wouldn‘t it be better to implement it on this side to be independent from the used package manager?
I really would love to see this feature. It would make things on our side much easier. If you can give me a hint I‘m willing to try to implement it.
@gkTim Sadly yes, Angular does not follow the best node module practices. I have hopes in ngLinker in the next Angular major release will allow us to solve this is a more elegant way. Personally, I have my own set of schematics extending from the official Angular schematics that do all this changes for me on project creation.
So I just started using rush a few days ago and I'm hitting one issue after another. But for me currently most issue come from linking the full source repo instead of just the "project/dist".
Not only this causes isses due to missing parts of the "uncompiled" packages.json, but also angular/ng are having issues like Function calls are not supported in decorators but 'InjectionToken' was called.
which magically go away once I manually replace the symlink from the package
folder with the package/dist
.
Is rush the wrong solution for building angular apps? If so, what would be the better option?
Can't you just add an option so I can manually overwrite the symlinking from package
to package/dist
?
if this is done via pnmp workspaces this feature would not be supported by npm or yarn, right?
Rush is getting out of the business of managing workspace linking, because all the package managers now have that feature built in. And because it is not a simple problem -- quite a lot of work was required to maintain Rush's implementation of useWorkspaces=false
in rush.json. Getting out of that business frees up resources so we can go solve other important problems that are NOT already addressed by package managers.
To answer your question, yes -- PNPM, Yarn, and NPM would need to each implement this feature. But it is a useful feature, and a good idea. It would be great if someone could find (or create) at PNPM issue requesting this feature, that we can refer everyone to.
Also, Angular needs to get with the times. :-) My understanding is that Google internally uses a highly proprietary build system, and then their OSS projects have to be adapted for usage outside Google, which results in setups that are not very well aligned with standard practice for OSS. Facebook has a similar problem (Watchman, HasteMap -- looking at you!) but they have done a good job of making React fit naturally into the ecosystem. React and Angular both began in the wild west, when nothing was standardized. But the JavaScript scene has come a lonnng way since then. It is no longer acceptable to pretend that monorepos don't exist, or that NPM is the only installation model.
Oh.. We've started to work with Rush just recently, and instantly bumped into that issue. Totally didn't expect Rush cannot work with such basic feature ._.
Summary
Add an optional
publishFolder
property to the project objects inrush.json
which Rush can use to link intra-project dependencies to and publish the project from. This option would allow package authors to postprocesspackage.json
before publishing, more easily prevent unnecessary files being uploaded to NPM, more easily support multiple flavors of the same release, and facilitate packages like@dojo/core
(which do not export their public API through amain
JavaScript file) linking between packages during development.Modifications
Add an optional
publishFolder
property inrush.json
:The above configuration would be used for a hypothetical project of three package directories that publish to four NPM packages. The build process for
packages/library
creates two flavors of the same release that publish to@my/library
and@my/library-plus
. The@my/consumer
package depends on@my/library
and the@my/consumer-plus
depends on@my/library-plus
. The link manager will need to be modified to point intra-package dependencies topublishFolder || projectFolder
. Similarly, the publish action will need to be modified to runnpm publish
inpublishFolder || projectFolder
. After linking, the project would have the following (partial) directory structure:Benefits
As mentioned before, several benefits come from this proposal:
src
and publish from a build directory can now properly reference modules within intra-package dependencies. For instance, the modulepackages/library/src/ModuleOne.ts
that builds topackages/library/release/ModuleOne.{js,d.ts}
and published as@my/library/ModuleOne
can now properly be referenced from modules withinpackages/consumer
as@my/library/ModuleOne
without using TypeScript'spath
configuration (which circumvents thenode
module lookup).