feat: pluggable widget engine with data-driven definitions#28
feat: pluggable widget engine with data-driven definitions#28engalar wants to merge 10 commits intomendixlabs:mainfrom
Conversation
Define WidgetDefinition, PropertyMapping, ChildSlotMapping, WidgetMode, and BuildContext types for data-driven widget construction. Implement OperationRegistry with 5 built-in operations (attribute, association, primitive, datasource, widgets) that wrap existing helper functions.
…dget engine Create 7 .def.json definition files (combobox, gallery, datagrid, 4 filters) that declaratively describe how MDL syntax maps to widget template properties. Add WidgetRegistry with embedded loading, case-insensitive lookup, and user-extension support (~/.mxcli/widgets/ and project-level overrides).
…e resolution Implements the generic build engine that constructs CustomWidget instances from WidgetDefinition + AST. Supports conditional mode selection (hasDataSource, hasAttribute, hasProp:X), property source resolution (Attribute, DataSource, Selection, CaptionAttribute, Association, generic), and child slot mapping.
…SP completion Wire up WidgetRegistry and PluggableWidgetEngine into pageBuilder with lazy initialization. COMBOBOX and GALLERY now route through the declarative engine via registry lookup in buildWidgetV3() default case. Remove 7 dead builder functions (~360 lines). Add `mxcli widget extract --mpk` command to generate skeleton .def.json from widget packages, and `mxcli widget list` to show registered widgets. LSP completion now includes registered pluggable widget types.
- Remove 5 dead .def.json files (datagrid + 4 filters) that were loaded but never used at runtime due to hardcoded switch cases - Change Modes from map[string]WidgetMode to []WidgetMode for deterministic evaluation order; add Name field to WidgetMode - Make LoadUserDefinitions report errors for malformed JSON and missing required fields instead of silently ignoring them - Add "selection" to OperationRegistry test coverage - Add TestOpSelection unit test for Selection field updates - Update combobox.def.json to array-based modes format
…entation Root cause: Gallery template was from a different widget version (33 properties) than the project's Gallery 1.12.0 (23 properties). Augmentation fixed property count but couldn't build nested ObjectType for object-type properties (filterList, sortList), causing CE0463 "widget definition changed" error. Changes: - Re-extract gallery.json template from Studio Pro reference (correct version) - Add Gallery to extract-templates command widget list - Add Children field to mpk.PropertyDef for nested object properties - Parse nested <properties> in mpk walkPropertyGroup - Build nested ObjectType in augmentation for object-type properties
Covers 4 test scenarios: - GALLERY basic with DATABASE datasource + TEMPLATE (PASS) - GALLERY with FILTER/TEXTFILTER (PASS) - COMBOBOX enum mode (known CE1613 engine bug) - COMBOBOX association mode (known CE1613 engine bug) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove dead DefaultSelection field from WidgetDefinition struct - Add warning logs for silent errors in initPluggableEngine and LoadUserDefinitions - Refactor TestOpSelection to test real opSelection function, add TestOpSelectionEmptyValue - Use return instead of break in buildWidgetV3 default case for clearer control flow - Extract association attribute correctly in ComboBox DESCRIBE (extractCustomWidgetPropertyAssociation) - Update MDL examples to reflect CE1613 fix status
…otes - Add engine design plan document - Add custom-widgets skill for Mendix projects - Add pluggable widget describe/parse tests - Add gallery user definition example - Add roundtrip issues tracking document
- Use atomic.Uint32 for placeholderCounter to prevent data races - Only ignore os.IsNotExist in loadDefinitionsFromDir, warn on other errors - Propagate registry init failure reason in unsupported widget type errors - Document LSP completion cache sync.Once limitation
ako
left a comment
ako
left a comment
There was a problem hiding this comment.
Code Review: feat: pluggable widget engine with data-driven definitions
25 files, +9385/-2662, 10 commits. Replaces hardcoded pluggable widget builders with a data-driven PluggableWidgetEngine using .def.json definition files.
Proposal Alignment
There are 5 relevant proposals in the repo. Here's how the PR maps to each:
| Proposal | PR #28 Alignment | Verdict |
|---|---|---|
MPK Widget Augmentation (docs/11-proposals/PROPOSAL_mpk_widget_augmentation.md) |
Directly implements — sdk/widgets/mpk/ parses .mpk files, augments templates at runtime |
Aligned |
BSON Schema Registry (docs/11-proposals/BSON_SCHEMA_REGISTRY_PROPOSAL.md) |
3-tier WidgetRegistry (embedded → global → project) matches the proposal's Tier 1/2/3 widget schema resolution | Aligned |
Show/Describe Pluggable Widgets (docs/03-development/proposals/show-describe-pluggable-widgets.md) |
Adds DESCRIBE-side association extraction for ComboBox, but the proposal's phased widget coverage (Image → Container → Value → Chart) is not addressed | Partially aligned |
Bulk Widget Property Updates (docs/11-proposals/PROPOSAL_bulk_widget_property_updates.md) |
Not addressed | N/A (separate scope) |
Update Built-in Widget Properties (docs/11-proposals/PROPOSAL_update_builtin_widget_properties.md) |
Not addressed | N/A (separate scope) |
Overall: Well-aligned with the MPK augmentation and BSON schema registry proposals. The data-driven .def.json approach with OperationRegistry is a clean implementation of the declarative widget construction concept.
Critical Issues
1. Association mapping runs before DataSource — wrong entity context (widget_engine.go)
In combobox.def.json, the association mode lists attributeAssociation (type: Association) before optionsSourceAssociationDataSource (type: DataSource). The engine processes mappings in definition order, so entityContext is stale when the association mapping runs — it uses whatever entity was set by the parent DataView, not the ComboBox's own datasource. The old hardcoded buildComboBoxV3 explicitly built the datasource first. This will serialize the wrong entity reference for ComboBox association mode.
2. Inconsistent gallery.def.json files
The embedded sdk/widgets/definitions/gallery.def.json has 2 child slots (TEMPLATE, FILTER) while .mxcli/widgets/gallery.def.json has 3 slots (TEMPLATE, EMPTYPLACEHOLDER, FILTERSPLACEHOLDER). Container name also differs: FILTER vs FILTERSPLACEHOLDER. Different behavior depending on which definition is loaded.
Moderate Issues
3. initPluggableEngine retries on every widget after failure (cmd_pages_builder.go)
If NewWidgetRegistry() fails, pluggableEngine stays nil but the guard only checks pluggableEngine != nil. Every subsequent pluggable widget re-attempts initialization, printing the warning repeatedly. Add a second guard: if pb.pluggableEngineErr != nil { return }.
4. LoadUserDefinitions error silently swallowed (cmd_widget.go)
_ = registry.LoadUserDefinitions(projectPath) discards errors. Users with malformed .def.json files get no feedback.
5. "Last fallback wins" instead of documented "first match wins" (widget_engine.go)
The selectMappings loop overwrites fallback on each no-condition mode instead of keeping the first one. Works by accident today (definitions only have one fallback), but violates the documented semantic.
Minor Issues
6. No validation of operation names at .def.json load time (widget_registry.go)
Invalid operation fields only fail at build time. Load-time validation would give better DX.
7. Test uses invalid condition string (widget_engine_test.go)
TestWidgetDefinitionJSONRoundTrip uses "DataSource != nil" which evaluateCondition doesn't recognize. Test passes because it only tests serialization, but embeds a wrong assumption about condition syntax.
8. Magic string "TEMPLATE" for default slot (widget_engine.go)
Hardcoded — should be a constant.
9. No zip-bomb protection in MPK extract (mpk.go)
Low severity for a CLI tool, but ParseMPK has no file size limits.
What Looks Good
- Architecture: OperationRegistry + WidgetDefinition + 3-tier WidgetRegistry is clean and extensible, matching the proposals well
- 361 lines of hardcoded builders replaced with 7 declarative
.def.jsonfiles — significant maintenance win mxcli widget extractCLI for generating skeleton definitions from.mpkfiles enables community contribution- Gallery CE0463 fix with proper nested ObjectType augmentation for object-type properties
- 40+ tests covering engine, registry, and DESCRIBE paths
- 3 self-review commits addressing dead fields, atomic counters, error propagation — good discipline
atomic.Uint32for placeholderCounter — correctly fixes a real race condition
Recommendation
Request changes — fix the critical mapping order dependency (#1) and resolve the gallery definition inconsistency (#2). The association/datasource ordering issue is a real correctness bug that will produce wrong BSON for ComboBox association mode. The rest can be follow-ups.
🤖 Generated with Claude Code
2 participants
Summary
cmd_pages_builder_v3_pluggable.go, 361 lines) with a data-driven PluggableWidgetEngine that uses.def.jsonwidget definition files~/.mxcli/widgets/), and project-level (.mxcli/widgets/)mxcli widget extractCLI command to generate skeleton.def.jsonfrom.mpkfilesTest plan
go test ./...)go vetclean🤖 Generated with Claude Code