siemens / meta-iot2000

SIMATIC IOT2000 Yocto Board Support Package
Other
127 stars 85 forks source link

Using pnpm for NodeRED #135

Closed bramroelandts closed 3 years ago

bramroelandts commented 5 years ago

Pnpm is a more performant version of npm. Instead of installing files in node_modules, it keeps a global store of dependencies, speeding up the whole process. It makes a significant difference in time required when installing a new NodeRED node on-target. The first time pnpm is run however, it needs to convert the npm file structure to its own structure.

In the Yocto environment, NodeRED is built using npm (using the script found in classes/npm-ng.bbclass). I was wondering how it would work to switch to pnpm from inside the Yocto environment. I thought I could try to modify npm-ng.bbclass, but apparently that script uses npm provided by nodejs-native, and not the host machine's npm.

If I want to use pnpm instead of npm from within the build env, do I need to create a recipe to build pnpm for the target machine first? And if I would manage that, would moving to pnpm from npm mean any real difficulties?

I do realize this will be quite some work, but the performance gain when installing packages on-target does justify this, I think.

Thanks!

jan-kiszka commented 5 years ago

Speeding up things would be interesting, of course. The critical question is, though, if all things will still be compatible.

IIUC, you will have to set a link to make "npm" point to "pnpm", otherwise node-red will not use that alternative package manager, right? Did you try that on the target device, installing pnpm first, then making maybe /usr/local/bin/npm point to /usr/bin/pnpm? What packages did you try to install afterwards?

If that pattern work, we can think about how to create pnpm and pnpm-native recipes and make sure that npm-ng uses that instead of native-npn. We would need pnpm both on the host, during build, as well as on the target device, obviously.

bramroelandts commented 5 years ago

IIUC, you will have to set a link to make "npm" point to "pnpm"

Well, not necessarily. After installing pnpm on the target device using these instructions, you can simply use pnpm install node-red-contrib-time-switch as you would normally use npm install node-red-contrib-time-switch inside the NodeRED directory (/usr/lib/node_modules/node-red).

There's no actual need to trick the system by pointing the npm executable to pnpm. As far as my testing has shown, pnpm and npm can live alongside each other. The only difference is that the first time pnpm runs, it restructures the node_modules folder.

We would need pnpm both on the host, during build, as well as on the target device, obviously.

At first, I thought it indeed would be needed on target and on host, but strangely enough I can build this repo using kas without having node and npm installed on my host system.

Pnpm and pnpm-native recipes seem the logical next step, but I'm unsure how that will work, since pnpm stores files in different directories compared to npm, so it does require some modification.

jan-kiszka commented 5 years ago

We cannot destroy the normal user story for installing nodes from within node-red. So, either we replace npm completely so that this still works, or we leave it up to expert users to do what you suggest.

bramroelandts commented 5 years ago

That's a fair point. I conducted some testing around this, and normally this shouldn't be a problem.

When a Node.js project is managed by pnpm and you try to run npm install xyz, it just installs xyz without any issues. If you afterwards run pnpm install again, it will convert that xyz packages to the pnpm structure.

So we could use pnpm by default and if people install nodes from within node-red, it won't cause any issues. Expert users can then use pnpm if they'd like to.

Simply tricking the system by pointing npm to pnpm seems a bit risky, as they don't share exactly the same syntax when it comes to advanced commands.

Where would I start if I'd like to write a recipe for pnpm?

jan-kiszka commented 5 years ago

I think node-red itself is not issuing advanced npm calls. So maybe we can trick it by setting a private PATH to a npn->pnpm wrapper when starting node-red. Then the rest of the system would continue to get npm when calling npm. But one after the other.

If you just want to package pnpm for the target you could try adding it via the npm-ng.class, i.e. install it via its npm package. Should be fairly analogue to the node-red package itself.

bramroelandts commented 5 years ago

I did try to package pnpm using npm-ng.bbclass, but that doesn't seem to work. The reason appears to be that the pnpm repo does not have a package.json file, which crashes npm-ng.bbclass.

In addition to packaging pnpm for the target, how would I go about creating a .bbclass file for it? Would it be a matter of duplicating npm-ng.bbclass and modifying it for pnpm?

jan-kiszka commented 5 years ago

The package you can pull from the registry does have a package.json. Don't use npm-ng on the source repo, it's not designed for that.

Regarding pnpm'ification during image build: you could derive a pnpm-class from npm-ng that appends to the the normal build the conversion step. That would allow sharing the not-so-simple fetcher which allows reproducible installations.

bramroelandts commented 5 years ago

The package you can pull from the registry does have a package.json

Fair point. It was actually the next line crashing the program. For some weird reason, pnpm's package.json does not have a dependencies field, but instead a __dependencies field. Even if I modify npm-ng.bbclass to use __dependencies, it crashes later on because no node_modules folder is found. For some reason, pnpm does not behave like a standard npm package.

Deriving a pnpm class by simply applying conversion after letting do npm its magic sounds like a rather good idea. I'll see what I manage to get working!

jan-kiszka commented 3 years ago

I suppose this topic won't be addressed for this layer anymore. If there is value and interest in improving the related class in meta-iot2050, discussion should be transferred.