- Issue created by @tedbow
- 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
See
core/modules/system/tests/modules/sdc_test/components/no-props/no-props.component.yml
for an example of a propless SDC.So yes, this is intended to work. And we should have an explicit test for it. 👍
- Assigned to Wim Leers
- Issue was unassigned.
- Status changed to Needs work
about 2 months ago 11:42am 23 July 2024 - 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
Test exists now. And it fails as expected.
Note that the proposed resolution presents
Check if the component has props defined before trying resolving them.
as the only possible choice.
But there's another possible choice: storing an empty list of props even for propless component instances. It keeps the structure simpler and non-conditional. I think that would make future component upgrade paths simpler (e.g. for adding props to a component that is already in use — which 📌 Ensure querying JSON nested values when parent keys are unknown is possible in all supported databases Active is laying the groundwork for).
The cost of that simplicity is slightly more storage space being consumed, because there's additional JSON to be stored.
- 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
Thanks to the draft MR I pushed, with clear
@todo
s, I think this is now actionable by novice contributors. - Assigned to tedbow
- 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
This is now blocking 📌 Create validation constraint for ComponentTreeStructure Needs work : https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...
- Assigned to Wim Leers
- Status changed to Needs review
about 1 month ago 7:40pm 29 July 2024 - Assigned to tedbow
- Status changed to Needs work
about 1 month ago 4:38pm 30 July 2024 - 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
📌 Create validation constraint for ComponentTreeStructure Needs work is in, can you rebase this? 🙏
- Assigned to Utkarsh_33
- Issue was unassigned.
- 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
Doesn't need to be @Utkarsh_33 who works on this :)
- 🇺🇸United States cosmicdreams Minneapolis/St. Paul
It sounds like what is left for this issue is to get it to pass the tests.
- Assigned to Wim Leers
- 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
With ~1 month of changes, I think a rebase is necessary.
- Issue was unassigned.
- 🇧🇪Belgium Wim Leers Ghent 🇧🇪🇪🇺
Merged in upstream, looks like more tests than before are failing.
- First commit to issue fork.
- Assigned to deepakkm