-
-
Notifications
You must be signed in to change notification settings - Fork 86
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
Question: Best practice for automating UI Builder dependency installation? #186
Comments
Hmm, interesting - are you on the latest version (v5.1.1)? Because that metadata should be getting updated whenever node-red starts up. the package-mgt.js startup function is called as uibuilder is initialised in NR startup. Line 120 updates the details both in-memory and in the package.json file. I wonder if that process is failing to update the file? |
...I was apparently testing with 5.0.2. Let me try with 5.1.1. Sorry, should've made sure I was on the latest version first. 🤦♂️ |
Not a problem, and I did make quite a few "improvements" in those processes, some of them probably even worked! |
🤣 It looks like my package.json had {
"name": "uib_root",
"version": "5.1.1",
"description": "Root configuration and data folder for uibuilder",
"scripts": {},
"dependencies": {
"vue": "^2.7.8"
},
"uibuilder": {
"packages": {}
}
} |
Hmm, OK. I fear you will need to increase the log level to Apologies for not doing this myself right now but My hand is in a pot (broken thumb). I can just about type but coding is too painful. |
Ouch, hopefully it makes for a good story at least! If I need to I can always try without Docker, but I'm currently at ~6G of space left on this laptop so I'm trying to keep things easily removable if possible. I can always test it on a pi if needed though. But maybe this is good enough for now, logs set to trace but still using docker:
Still not populated: $ cat /data/uibuilder/package.json
{
"name": "uib_root",
"version": "5.1.1",
"description": "Root configuration and data folder for uibuilder",
"scripts": {},
"dependencies": {
"vue": "^2.7.8"
},
"uibuilder": {
"packages": {}
}
} |
I think this may be (at least part of) the issue, haven't done any further testing: node-red-contrib-uibuilder/nodes/libs/package-mgt.js Lines 297 to 299 in 3911282
Small test: async function updIndividualPkgDetails(pkgName, lsParsed) {
return new Promise(resolve => {
setTimeout(() => {
lsParsed.dependencies[pkgName] = 'done';
resolve();
}, 500);
});
}
async function forEach() {
const lsParsed = { dependencies: { vue: '^2.7.8', test: '1.0.0' } };
await Object.keys(lsParsed.dependencies || {}).forEach( async pkgName => {
await updIndividualPkgDetails(pkgName, lsParsed);
console.log('updated package:', pkgName);
});
console.log(lsParsed);
}
async function promiseAll() {
const lsParsed = { dependencies: { vue: '^2.7.8', test: '1.0.0' } };
await Promise.all(
Object.keys(lsParsed.dependencies || {}).map(async (pkgName) => {
await updIndividualPkgDetails(pkgName, lsParsed);
console.log('updated package:', pkgName);
})
);
console.log(lsParsed);
}
promiseAll();
// forEach(); Promise.all():
forEach:
|
Ah, I hate await! Don't suppose you'd like to provide a PR? The main branch can be used and I can issue an updated version. |
Yeah sure I could definitely do that. Probably not until tomorrow though, got busy with some other stuff today. |
Hmm seems there's more to it than that. Changing to I'll have to play around with it a bit more when I have some time to find the root cause and provide a worthwhile PR. |
I think, on further reading, this may have the answer: https://gist.github.com/joeytwiddle/37d2085425c049629b80956d3c618971 |
If you get a chance to try it. Could you please try
That should process all of the package dependencies in parallel. |
Hmm still doesn't seem to fix it, logs look about the same (with an empty {} for bootstrap-vue's npmLatestVersion log?):
and package.json is still only partially filled out like before:
I believe the empty |
I'm thinking |
It does. The code is rather convoluted I'm afraid. Not sure what I was drinking at the time! But it sets The fundamental issue is that the npm checks HAVE to be done async otherwise the whole node-red startup is delayed by at least 2-3 seconds. BUT, that starts an async promise chain that is very hard to stop. JavaScript promises are just plain stupid. So one thing to check. WHEN are you checking the package.json file? It does take an appreciable amount of time for the file to get updated. Especially if you have a lot of libraries installed. On my fast dev system with just 4 libraries, it still takes around 2-3 seconds. Just checking the code again - there are some really funky things going on after a few adjustments. I suspect that the whole package handler needs a serious re-write. I'm going to have to go through the whole process line by line I'm afraid. |
Looks like if you start with an empty So I need to find what is emptying the object again. |
OK, so the npm ls command takes far too long so I'm trying to create the minimal version of the data on startup. Just enough to satisfy the web startup (where the libraries are added to the vendor routes to make them available to the front-end. The following manual changes should work: In
and after Then in
Hopefully that should work now. The package.json file is still written to twice but from different places now. Firstly to get the simplistic entries for web.js and then again for the full data needed for the libraries tab in the Editor (which is the bit that takes the time and has to be async). I'll be pushing these changes to the v6 branch shortly. I need to do some more tidying up since I think there are now several functions in package-mgt.js that are no longer needed. |
Thanks for taking the time to look into this. I made these changes and it looks like package.json gets updated as it should now, but I'm still getting the ? at the end of the "Est link". It also seems like this somehow broke installing packages through the editor UI. I installed one through your UI, it says it completed but there's no new row added to the package list in the UI. The package.json has it listed as a |
Drat. It is installing but not updating the list in the editor again. Another fix needed. If you remove a package, it is removed but also not being removed from the list. |
…ndency installation? #186 Hopefully complete now
…ndency installation? #186 Hopefully complete now
OK, another set of changes. This time to To be honest, you would now be better off trying out the v6 branch direct from github which is where all the changes are. But if you want to carry on with your own patched version, you should be able to derive patch versions from the commits I think. |
I can try your v6 branch tomorrow so we're on the same page, just been a bit distracted with other stuff today. |
Seems like the UI install is working again now, still ending up with a ? though for my manually installed dependencies. So progress at least. I'll try to play around with it a bit more when I have a chance. |
Stale issue message |
Sorry I haven't had a chance to get back to this for a while. Feel free to close this if you'd like as it's not a huge issue, just takes an extra minute to manually install dependencies. When I get a chance I can hopefully try to take a look at this again sometime. |
Thanks, I'm pretty sure every thing is working again now in the v6 branch. I need to get back to working on it. I've had a bit of a break. Thanks for your patience. I'll close this but if you discover any other issues, please do let me know. |
Confirmed that manually installed packages in your uibRoot folder correctly populate all of the metadata after node-red is restarted. |
We're using docker to run Node-RED. Currently we have uibuilder getting installed automatically by providing a package.json to Node-RED, per these instructions https://nodered.org/docs/getting-started/docker:
This gets us halfway there with node-red's dependencies, but then we still need to manually install uibuilder dependencies (vue, etc.) through the admin interface after starting Node-RED.
I did some testing and it seems like we can provide a minimal /data/uibuilder/package.json file and run an
npm install
in the Dockerfile and everything works for the most part. ("PACKAGE vue NOT FOUND" error if we provide package.json but don't run annpm install
before starting)Dockerfile which installs uibuilder and its dependencies, sets up minimal index.html for testing:
Minimal test flows.json:
Node-RED package.json:
Minimal uib-package.json:
The only issues I see doing it this way is that UI Builder isn't updating the package.json's
uibuilder.packages
, only creating an empty stub:uib-package.json after starting Node-RED:
which results in an incorrect/missing "Est. link" in the admin interface:
This isn't really an issue though since our actual index.html already has the correct paths for the dependencies so this is just a small visual issue as far as I can tell. The file itself is still available where it should be at http://localhost:1880/uibuilder/vendor/vue/dist/vue.js, and no errors are logged.
So I guess my question is, is this the most correct/reliable way to be doing this?
npm install
at the build stage from the Dockerfileuibuilder.packages
entry in package.json as it seems to only cause visual issues and no actual errors?I don't really like the idea of providing a package.json with a pre-populated
uibuilder.packages
, since that seems like something that may change over time (like it just did in the last major release!) and should be handled internally by uibuilder instead so we don't have to keep up with changes to the structure of this file. Are there any hidden issues I may have missed that could be introduced by installing uibuilder dependencies this way? Or any planned changes to uibuilder that may break this in the future?The text was updated successfully, but these errors were encountered: