smapiot / piral

πŸš€ Framework for next generation web apps using micro frontends. ⭐️ Star to support our work!
https://piral.io
MIT License
1.68k stars 125 forks source link

Piral with Rush #667

Closed KBroichhausen closed 7 months ago

KBroichhausen commented 8 months ago

Bug Report

For more information, see the CONTRIBUTING guide.

Prerequisites

Environment Details and Version

Windows 10, pnpm 7.33.5, npm 6.14.15, Node.js 18.17.1, Rush 5.122.2

Description

I try to get a working example of Piral with Rush and following the tutorial https://docs.piral.io/guidelines/tutorials/23.2-monorepo-rush I'm not able to get it.

Let's start with NPM as package manager:

NPM

Do the git and rush init. Change to npm in rush.json. Add the shell with the npx piral new ... command. Check the rush.json if the project was added. Use rush update to install packages and finally call rush build.

PS C:\Projekte\piral-npm> rush build

Rush Multi-Project Build Tool 5.112.2 - https://rushjs.io
Node.js version is 18.17.1 (LTS)

Starting "rush build"

...

==[ FAILURE: 1 operation ]=====================================================

--[ FAILURE: app-shell ]-------------------------------------[ 0.08 seconds ]--

node:internal/modules/cjs/loader:1080
  throw err;
  ^

Error: Cannot find module 'C:\Projekte\piral-npm\packages\app-shell\node_modules\piral-cli\lib\piral-cli.js'
    at Module._resolveFilename (node:internal/modules/cjs/loader:1077:15)
    at Module._load (node:internal/modules/cjs/loader:922:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:23:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

Node.js v18.17.1

Operations failed.

rush build (0.10 seconds)

Hmmm, OK, check the node_modules folder. As you can see in the screenshot, it doesn't install the devDependencies in the folder and therefore it's missing piral-cli. grafik

Check the rush temp folder of modules in common\temp\node_modules: This contains the piral-cli but with the way how resolving works for modules, it of course cannot be found there.

Do something you shouldn't do: npm install in the app-shell folder. Now building works.

Add a pilet:

PS C:\Projekte\piral-npm\packages\app-shell> npx pilet new app-shell --no-install --target packages/foo-pilet --npm-client rush
+ piral-cli v1.4.3
βœ– [0002] Could not install the package "app-shell@latest" using rush. Make sure rush is correctly installed and accessible: Error: npm WARN deprecated querystring@0.2.1: The querystring API is considered Legacy. new code should use the URLSearchParams API instead.
npm ERR! code EUNSUPPORTEDPROTOCOL
npm ERR! Unsupported URL Type "workspace:": workspace:*

...

I mean, things are changing pretty fast but afaik workspace:* is something working only when using pnpm. I double checked and my rush.json and set up was all done with:

  // "pnpmVersion": "7.33.5",

  "npmVersion": "6.14.15",
  // "yarnVersion": "1.9.4",

The two issues are very crucial, so I stopped trying it with npm and changed to pnpm.

PNPM

Everything is working fine until try to build the app-shell:

rush build
...
- Reading configuration ...
- Starting emulator build ...
β„Ή Bundle emulator ...
⚠ [0176] Installing default bundler since no bundler has been found.
- Installing piral-cli-webpack5 ...
βœ– [0002] Could not install the package "piral-cli-webpack5@^1" using rush. Make sure rush is correctly installed and accessible: Error

No problem just do rush add -p piral-cli-webpack5 --dev in the app-shell folder and it works.

Now, add a pilet:

npx pilet new app-shell --no-install --target packages/foo-pilet --npm-client rush
+ piral-cli v1.4.3
βœ– [0002] Could not get externals from "piral": "Error: Can't resolve 'piral/package.json' in 'C:\Projekte\piral\packages\foo-pilet'
βœ– [0002] Could not get externals from "piral-core": "Error: Can't resolve 'piral-core/package.json' in 'C:\Projekte\piral\packages\foo-pilet'
βœ– [0002] Could not get externals from "piral-base": "Error: Can't resolve 'piral-base/package.json' in 'C:\Projekte\piral\packages\foo-pilet'
⚠ [0078] Did not find any reference to either piral, piral-core, piral-base. No framework dependencies are shared.
βœ” Pilet scaffolded successfully!

Hmmm lots of errors. Let's just try to install the packages:

rush build
Progress: resolved 549, reused 540, downloaded 0, added 0, done
../../packages/foo-pilet postinstall$ pilet declaration
β”‚ + piral-cli v1.4.3
β”‚ Cannot find module 'app-shell/package.json'
β”‚ Require stack:
β”‚ - C:\Projekte\piral\common\temp\node_modules\.pnpm\piral-cli@1.4.3\node_modules\piral-cli\lib\common\declaration.js
β”‚ - C:\Projekte\piral\common\temp\node_modules\.pnpm\piral-cli@1.4.3\node_modules\piral-cli\lib\common\index.js
β”‚ - C:\Projekte\piral\common\temp\node_modules\.pnpm\piral-cli@1.4.3\node_modules\piral-cli\lib\plugin.js
β”‚ - C:\Projekte\piral\common\temp\node_modules\.pnpm\piral-cli@1.4.3\node_modules\piral-cli\lib\start.js
β”‚ - C:\Projekte\piral\common\temp\node_modules\.pnpm\piral-cli@1.4.3\node_modules\piral-cli\lib\select.js
β”‚ - C:\Projekte\piral\common\temp\node_modules\.pnpm\piral-cli@1.4.3\node_modules\piral-cli\lib\pilet-cli.js
β”‚ + Find detailed descriptions at https://docs.piral.io/code/search
└─ Failed in 688ms at C:\Projekte\piral\packages\foo-pilet
 ELIFECYCLE  Command failed with exit code 1.

This is a combination of problems in the postinstall script pilet declaration with how resolving packages, pnpm and declarations.js works. declarations.js is calling const path = require.resolve(`${piralInstance}/${constants_1.packageJson}`); Resolving just traverses recursively the parent dirs and tries to find the module in a node_modules folder. Omitting some user dirs and global, it looks like this for me:

[
  'C:\\Projekte\\piral\\common\\temp\\node_modules\\.pnpm\\piral-cli@1.4.3\\node_modules\\piral-cli\\lib\\common\\node_modules',
  'C:\\Projekte\\piral\\common\\temp\\node_modules\\.pnpm\\piral-cli@1.4.3\\node_modules\\piral-cli\\lib\\node_modules',       
  'C:\\Projekte\\piral\\common\\temp\\node_modules\\.pnpm\\piral-cli@1.4.3\\node_modules\\piral-cli\\node_modules',
  'C:\\Projekte\\piral\\common\\temp\\node_modules\\.pnpm\\piral-cli@1.4.3\\node_modules',
  'C:\\Projekte\\piral\\common\\temp\\node_modules\\.pnpm\\node_modules',
  'C:\\Projekte\\piral\\common\\temp\\node_modules',
  'C:\\Projekte\\piral\\common\\node_modules',
  'C:\\Projekte\\piral\\node_modules',
  'C:\\Projekte\\node_modules',
  'C:\\node_modules',
]

But pnpm symlinks the workspace:* dependencies only to the projects node_modules dir. In this case in packages/foo-pilet/node_modules/app-shell/.

Steps to Reproduce

Just follow the tutorial https://docs.piral.io/guidelines/tutorials/23.2-monorepo-rush

Expected behavior

A working example ;)

Actual behavior

See description.

Possible Origin/Solution

For NPM: It looks like rush is creating the node_modules folder in a wrong way but after that it looks like piral creates a wrong package.json for rush? Don't really know which part of the stack is responsible for it.

For PNPM: First errors when adding, I have no idea. Second error: Maybe piral-cli needs an additional parameter to point to the pilet's folder? That way it would see the symlinked packages pointing to other packages of the monorepo added via workspace:*.

FlorianRappl commented 8 months ago

I am not sure if we have the capacity to do that.

Either its picked up by the community or we will abandon Rush. The fact is that Rush was already an outdated solution back then and I am not sure if they managed to have their solution working / what they did in the meantime. That being written if you want to look into it and make it work again it would be superb; otherwise I'd just remove Rush if it stopped working with recent versions.

KBroichhausen commented 8 months ago

Thanks for the quick response. Hmmm I'm looking through various microfrontend frameworks and one of many good reasons for piral is the support for rush as rush is already in use as monorepo.

NPM and rush is nothing really working good together. The way to go is probably remove Rush+NPM and only support Rush+PNPM.

I looked a little bit into the issues:

1. βœ– [0002] Could not install the package "piral-cli-webpack5@^1" using rush. Make sure rush is correctly installed and accessible: Error at the app-shell: The problem is that the standard workflow is to call rush build which just executes the build script defined in the package.json => piral build. During the build it tries to install packages which does not work. It is only allowed to run one rush command in a workspace otherwise you get

Another Rush command is already running in this repository.

That's not 100 % true because there are some commands running without locks. Also custom commands can use safeForSimultaneousRushProcesses.

2.

βœ– [0002] Could not get externals from "piral": "Error: Can't resolve 'piral/package.json' in 'C:\Projekte\piral\packages\foo-pilet'
βœ– [0002] Could not get externals from "piral-core": "Error: Can't resolve 'piral-core/package.json' in 'C:\Projekte\piral\packages\foo-pilet'
βœ– [0002] Could not get externals from "piral-base": "Error: Can't resolve 'piral-base/package.json' in 'C:\Projekte\piral\packages\foo-pilet'

If the packages have been added before into the package.json and properly installed, there would be no need to resolve it by hand because pnpm already takes care of not having package duplicates by symlinking.

3. Finding the pilet's app-shell is easy patchable by just adding the root path to the modules path. https://github.com/smapiot/piral/blob/6429391c1b248272ec414462e174144932bc6b3e/src/tooling/piral-cli/src/common/declaration.ts#L147-L157 add module.paths.push(join(root, "node_modules")); where join comes from path.

4. Something new while playing around with it. If I fix 1-3 by hand, as soon as I add the pilet to the monorepo, the build for the shell doesn't work again. It tries to always installs the bundler. The only thing I have seen that withBundler is not called anymore with the webpack5 string but I don't really know what calls it and how this function is called. Guess something with the inject of the plugin is going wrong.

FlorianRappl commented 8 months ago

The only reason for a bundler to be installed is that the / no bundler is not available - being available means it can resolve it / has it installed as a piral cli plugin.

So this will have to do with the structure that Rush makes when using npm (if they would just use npm workspaces the problem would be easily solved, but I think they do something on their own).

KBroichhausen commented 8 months ago

being available means it can resolve it / has it installed as a piral cli plugin.

That was the part I'm struggling with. It finds the plugin when only have one project in rush but not when I have two projects in rush. Looking into the README and source for the plugins, it looks like the plugins are found / searched for by using a magic naming convention piral-cli-. Guess it's once again a wrong search path in that case.

This should be fixable.

Just don't know what to do about the part that piral tries to install things during a rush build (which just calls the build scripts of package.json -> piral build). Not the best user experience but I can only think about throwing a message to the user telling that xyz needs to be installed manually via rush add ....

FlorianRappl commented 8 months ago

Looking into the README and source for the plugins, it looks like the plugins are found / searched for by using a magic naming convention piral-cli-. Guess it's once again a wrong search path in that case.

Yes, I'll also try to look into it today. Do you have a reproducible / some repo somewhere that simplifies the setup for me? Would be appreciated. Thanks!

Just don't know what to do about the part that piral tries to install things during a rush build (which just calls the build scripts of package.json -> piral build)

Actually it should not. There should be just one exception to this: If a bundler is not installed. Otherwise it cannot function - which is a worse experience. So I think resolving the plugin lookup will also resolve this one.

KBroichhausen commented 8 months ago

https://github.com/KBroichhausen/rush-piral

Probably the thing here needs a different solution to find the dir to also work with rush + pnpm. https://github.com/smapiot/piral/blob/develop/src/tooling/piral-cli/src/plugin.ts#L116C1-L123C2

For me it searches in common\temp\node_modules\.pnpm\piral-cli@1.4.3_rxrtjl6pgrx2bg5f4pwvyscc3u\node_modules but it would require to look for it in common\temp\node_modules\.pnpm\ or much better and more convenient in the app's node_modules folder which is packages\app-shell\node_modules

Otherwise it cannot function - which is a worse experience. So I think resolving the plugin lookup will also resolve this one.

Yeah, unless you really have no bundler installed. In that case you can only throw a message to the user.

FlorianRappl commented 8 months ago

Well I mean you use pnpm, so the app's node_modules folder is just a bunch of symlinks. Actually, for pnpm we already have a bit of special treatment in there. Let me see with your repo what we can do here to improve this.

FlorianRappl commented 8 months ago

Alright so I got it working. There are two problems:

  1. It's a path resolution issue; apparently (maybe with a more recent version of pnpm) there are sometimes / under to me unknown conditions paths generated that are not just containing something like /piral-cli@1.0.0/ but like /piral-cli@1.0.0_abc123. I've introduced some logic handling this
  2. The pilet does not have a bundler set up (something like "piral-cli-webpack5": "1" as the app shell does would be sufficient).

For (1) I'm pushing right now. I will now also look into making this work with npm. Also I will update the docs to explicitly mention Rush 5 - just to have this stated in case of future versions of Rush etc.

Also quick note: I am on Mac and I had to change the rush config from the app shell to be packages/app-shell instead of packages\app-shell. I guess the former should also work on Windows and would be more cross-platform compatible. But for Piral (as it normalizes the paths) it should not make a difference.

KBroichhausen commented 8 months ago

It's a path resolution issue; apparently (maybe with a more recent version of pnpm) there are sometimes / under to me unknown conditions paths generated that are not just containing something like /piral-cli@1.0.0/ but like /piral-cli@1.0.0_abc123. I've introduced some logic handling this

Yikes! That one is on me. It does that because it is a patched dependency. See the note on the bottom of the README of my repo.

If you have a look at the patch, you may want to include (at least logically) the part in the TS file to make the post-install script for pilets in rush monorepo work.

EDIT: That also explains why it worked if only the app-shell was in the mono repo: I didn't had to patch the dependency for pilet and so the code to resolve the plugins in pnpm node_modules path was working.

FlorianRappl commented 8 months ago

Yeah I also now recognize the patch; I think this also needs to be patched in the source (though a bit differently). Currently on that.

(But the changes so far are good anyway; now its also stable in pnpm even when patched.)

FlorianRappl commented 8 months ago

With the recent patch (version currently in build) it should work in pnpm and others (npm).

KBroichhausen commented 8 months ago

Nice. Thanks a lot!

KBroichhausen commented 8 months ago

A follow up on this: How I can run / watch multiple pilets locally? I had a look how it is achieved in the Admin UI piral sample and it is doing it via pilet debug 'packages/pilet-*/src/index.tsx but doing this in a rush monorepo once again gives me the no bundler found message and also isn't able to install one. Sounds again like an issue with resolving the modules.

FlorianRappl commented 8 months ago

I am not sure what pilet refers to in your machine / your repo. In the given monorepo its using npm workspaces, so any binary is also available workspace-wide. Either install the Piral CLI in the root already (I don't think Rush allows this / uses this concept) or just start the build from one of the pilets (you can always pilet debug . ../foo ../bar etc.).

KBroichhausen commented 8 months ago

I use the version with the recent changes for rush:

pilet --version
1.5.0-beta.6569

pilet is located in %APPDATA%\npm\pilet.ps1

pilet debug . ../foo ../bar doesn't work. It finds the bundler for the . pilet but not for the others like ../foo. That's why I guess the search path is wrong again when passing folders.

Though it works if I add this to the scripts section in a package.json of one pilet and then call it via npm run ... or pnpm ... or rushx ... etc. Well, it works but as rush, and you already mentioned by yourself, has no concept of a root package.json, you need to add it to one pilet. If it's not easy fixable, then it should go into the Guidelines. I guess I can rewrite and be more precise in some parts of the Guideline and make a PR in the near future for it.

FlorianRappl commented 8 months ago

Well this means you installed the Piral CLI globally. Hence you should also have installed a bundler globally.

In general we don't recommend a global installation - but in this case (i.e., using Rush, which, again, is a bit of a problematic / imho not well done monorepo tool) you'd need it to run it from the root.