Skip to content
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

load preinstalled 3rd party widgets #1274

Conversation

theotherwillembotha
Copy link

Description

We build a docker image based on the official nodered docker image and and bundle it with a couple of our own plugins and widgets. The current 3rd party widget loader code only loads widgets from the package.json file in the user settings folder and not from the package.json file in the nodered root /base where preinstalled nodered plugins reside. This changeset allows widgets to be installed from both locations in addition to loading plugins relative to a node_modules folder or package.json file.

Related Issue(s)

Issue 1159

Checklist

Testing requires loading 3rd party plugins at runtime which the current testing framework doesn't support.

  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts

No documentation changes needed

  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production

No changes to flowforge.yml

Labels

  • Includes a DB migration? -> add the area:migration label

@joepavitt joepavitt requested review from joepavitt and Steve-Mcl and removed request for joepavitt September 11, 2024 10:01
@Steve-Mcl Steve-Mcl mentioned this pull request Sep 11, 2024
10 tasks
@Steve-Mcl
Copy link
Contributor

hi @theotherwillembotha

Thank you very much for this contribution. It is very much appreciated.

You may notice that we have raised a new PR that supersedes this: #1286

Your contribution was fine however I felt it important that unit tests were present with this PR and that meant refactoring. So to avoid a lot of too-and-fro, I ended up just raising a new PR. I hope you dont mind.

FYI, there were a couple of issues that would have needed to be fixed before merging as it stood:

  1. linting errors (use of semi-colons, spacing, etc) were present (we use eslint in our GH integrations and local env)
  2. Use of process.cwd() to locate main package would not be suitable for all types of node-red installations (i.e. launching it from a different directory)
  3. Did not support nodesDir
    4. Since we are touching this code, we might as well support loading nodes that Node-RED loads via nodesDir
  4. No unit tests present - this one felt important in this area (so as to avoid future regressions)

I hope this doesnt put you off contributing further features 🤞

thanks again, Steve.

@Steve-Mcl Steve-Mcl closed this Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants