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

Node property typing #37

Merged
merged 2 commits into from
Jan 8, 2021
Merged

Node property typing #37

merged 2 commits into from
Jan 8, 2021

Conversation

knolleary
Copy link
Member

@knolleary knolleary commented Sep 29, 2020

A node's defaults object in its HTML definition provides a list of its properties with some metadata associated with them.

If a property is intended to be a reference to a configuration node, its entry in the defaults object will include the type property that identifies the type of config node it should point to.

This allows the editor to automatically generate the Config Node select box UI and manage the relationship between the nodes.

This design note explores extending this property to allow for a richer set of relationships between nodes.

It does not (currently) talk about specifying types of node properties more generally - this is only where there is a node property that holds the id (or multiple ids) of other nodes. The editor needs this information when importing/exporting nodes to ensure the references are preserved if ids are changed.

This is a very initial design note just to get these ideas out of my head and somewhere we can discuss them. There is more work to be done around what (if any) UI improvements this could lead to around Config node selection in a node's edit dialog.

@KazuhiroItoh
Copy link
Contributor

In the [UI Module] function (#24), I would like to add references to subflow templates and instances.
subflow: {type: "subflow"} ・・・ Subflow template
subflow: {type: "subflow:"} ・・・ Subflow instance
In addition, I believe that widgetOrder: { type: ":any[]"} can be used in the widget array.

Should I base our support for these features on the considerations in this design note?
Or can I propose to incorporate the same functionality in the existing implementation?

First of all, we have considered supporting these features in the existing implementation.
Reference implementation: node id replace for subflow and widgets

If there is no problem, I would like to issue a PR.
Please check.
Thank you and best regards.

@knolleary
Copy link
Member Author

I see from the commit you link to, you are hardcoding the name of Node-RED Dashboard node type into the core of Node-RED - we cannot do that. It is also the exact scenario this Design note is intended to fix.

Should I base our support for these features on the considerations in this design note?

If the ui module design requires a node's property to reference another node by ID, then yes, this design note is relevant and becomes a prerequisite to the ui module work.

This design is still in draft state - it was written to capture the idea for the future, not something I am actively working on. But if this is now needed by the ui_module work, then it needs to be completed and implemented in full.

As you can see, there have been no comments on the design I've written - it would be nice to get some feedback to know if this design makes sense and whether it is missing anything.

@KazuhiroItoh
Copy link
Contributor

As you can see, there have been no comments on the design I've written - it would be nice to get some feedback to know if this design makes sense and whether it is missing anything.

I understand.
I will consider the correspondence with this design note.

knolleary added a commit to node-red/node-red that referenced this pull request Jan 8, 2021
@knolleary knolleary marked this pull request as ready for review January 8, 2021 14:39
@knolleary
Copy link
Member Author

This Design is complete and initial support implemented via node-red/node-red#2812

There is some follow-up design work about what UI features can now be enhanced with this type-declaration in place.

For example, the UI currently displays a select box when a property is declared as being a single type that is a config node. That can now be updated to support the "foo | bar" style syntax and present a list of both valid types.

@knolleary knolleary merged commit 3699a83 into master Jan 8, 2021
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.

2 participants