Nested SystemTypes: Add 'parent' to SystemType, update SystemType discovery, add tests #457
Conversation
AntoineGautier
left a comment
AntoineGautier
left a comment
There was a problem hiding this comment.
@darenkeck-dev See my comments:
- The ES version should be updated if we want to use the
at()method. - The other one is more a question related to the requirement to organize templates by library.
| systemTypeStore.set(systemType.modelicaPath, systemType), | ||
| ); | ||
| // set the systemType reference on the template | ||
| this.systemTypes.unshift(systemTypes.at(-1)!); |
There was a problem hiding this comment.
The at() method was introduced in ES2022, but server/tsconfig.json is targeting ES2019.
There was a problem hiding this comment.
Thank you, good catch. This change should be safe the node version we are using for the parser.
There was a problem hiding this comment.
Pushed a commit specifying ES2022 both for the client and server.
| const systemType = { | ||
| description: type.description, | ||
| modelicaPath: type.modelicaPath, | ||
| parent: parent?.modelicaPath || "", |
There was a problem hiding this comment.
If we want to organize templates by library as shown below, don't we need to store the Modelica library as the parent of each top-level system type?
├── Library 1
│ └── Air handlers and fans
│ └── Plants
└── Library 2
└── Air handlers and fans
└── Zone equipment
There was a problem hiding this comment.
Thank you, this is an important point.
Do you know if we have this information already available, that overall package details get extracted when we go through the template discovery process?
There was a problem hiding this comment.
Actually, we don't have this information yet.
I think if we change the following line:
https://github.com/lbl-srg/ctrl-flow-dev/blob/main/server/src/parser/loader.ts#L133
into:
while (packageName) {we will have the root package details stored as an additional TemplateNode.
There was a problem hiding this comment.
Removed the rootPackageName part of the condition and tested both the parser and interpreter and both are behaving well. Also, updated parser tests impacted by this change.
|
@AntoineGautier changes have been made hiding any visible update in the client UI. This is ready for review again. |
Description
Server updates to support nested SystemTypes.
parentattribute to SystemTypeparentfieldClient
NOTE: no visible updates have been made in the client UI. This PR merges work to prepare for displaying nested templates and system type groups.
Related Issue(s)
#455
Testing
Tests have been updated on the server