FE2 DS modules & single load path for DS and control plugins - #8852
Merged
sumerjabri merged 7 commits intoAug 4, 2026
Conversation
…or load path for DS and control plugins. Controls consume resolved actions/capabilities instead of switching on DS type; plugin bundles register via importPlugin/registerPlugin. Also fixes Create on path-only shared-content and content-types resolution loops.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe Forms Engine now uses shared plugin descriptors, data-source registries, typed action resolution, built-in modules, plugin control loading, descriptor-defined additional fields, and specialized controls. ChangesForms Engine plugin and data-source convergence
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
studio-ui/ui/app/src/components/FormsEngine/FormsEngine.tsx (1)
605-621: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd
customControlsto the effect dependency array.When
fetchSiteUiConfigCompletereplacesstate.uiConfig.controlsafter mount, rerun the effect. Otherwise, custom-control additional fields can lack their atoms.importPluginandregisterPluginupdate plugin registries, notstate.uiConfig.controls.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio-ui/ui/app/src/components/FormsEngine/FormsEngine.tsx` around lines 605 - 621, Add customControls to the dependency array of the surrounding FormsEngine effect so it reruns when state.uiConfig.controls is replaced after mount. Preserve the existing dependencies and use the customControls value already derived in the component; do not rely on importPlugin or registerPlugin registry updates.
🧹 Nitpick comments (11)
studio-ui/ui/app/src/components/FormsEngine/lib/valueRetrievers.ts (1)
121-122: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the duplicated
customControls/controlDescriptorsmerge into a shared helper. Both files independently compute{ ...customControls, ...controlDescriptors }to resolve the descriptor lookup table, including an unresolved TODO about merge priority in one of them. One shared helper (e.g.resolveControlDescriptors(customControls)) keeps the merge order consistent if it ever needs to change.
studio-ui/ui/app/src/components/FormsEngine/lib/valueRetrievers.ts#L121-L122: replace the inline{ ...customControls, ...controlDescriptors }and its TODO comment with a call to the shared helper.studio-ui/ui/app/src/components/FormsEngine/lib/formUtils.tsx#L880-L897: replace the inline{ ...customControls, ...controlDescriptors }with the same shared helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio-ui/ui/app/src/components/FormsEngine/lib/valueRetrievers.ts` around lines 121 - 122, Extract the duplicated customControls/controlDescriptors merge into a shared resolveControlDescriptors helper, preserving the current merge order. In studio-ui/ui/app/src/components/FormsEngine/lib/valueRetrievers.ts lines 121-122, replace the inline merge and TODO with the helper call; make the corresponding replacement in studio-ui/ui/app/src/components/FormsEngine/lib/formUtils.tsx lines 880-897, importing or defining the shared helper as appropriate.studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/audioBrowseRepo.ts (1)
26-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated repo-path fallback logic across three built-in data-source modules. The
propString(record, 'repoPath') || propString(record, 'path', '/static-assets/')fallback is copied verbatim in three files; extracting a shared helper inmoduleHelpers.tsremoves the duplication and keeps future fallback changes in one place.
studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/audioBrowseRepo.ts#L26-L41: replace the inline fallback on Line 27 with a sharedresolveRepoPath(record, '/static-assets/')helper.studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/audioDesktopUpload.ts#L26-L41: replace the inline fallback on Line 27 with the same shared helper.studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/videoDesktopUpload.ts#L26-L41: replace the inline fallback on Line 27 with the same shared helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/audioBrowseRepo.ts` around lines 26 - 41, Extract the shared repo-path fallback into resolveRepoPath in moduleHelpers.ts, preserving repoPath, path, and the '/static-assets/' default precedence. Replace the inline fallback in create in studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/audioBrowseRepo.ts lines 26-41, audioDesktopUpload.ts lines 26-41, and videoDesktopUpload.ts lines 26-41 with resolveRepoPath(record, '/static-assets/').studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/imgDesktopUpload.ts (1)
27-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated repo-path fallback into
moduleHelpers.ts. All three sites resolve a module's repository path with the identical expressionpropString(record, 'repoPath') || propString(record, 'path', '/static-assets/'); the shared root cause is that this fallback logic has not been extracted into a helper alongsidepropString/propBoolean.
studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/imgDesktopUpload.ts#L27: replace the inline expression with a shared helper, e.g.resolveRepoPath(record, '/static-assets/').studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/imgRepositoryUpload.ts#L27: replace the inline expression with the same shared helper.studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/videoBrowseRepo.ts#L27: replace the inline expression with the same shared helper.♻️ Proposed helper addition
// in moduleHelpers.ts +export function resolveRepoPath(record: DataSourceRecord, fallback: string): string { + return propString(record, 'repoPath') || propString(record, 'path', fallback); +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/imgDesktopUpload.ts` at line 27, Extract the repeated repository-path fallback into a shared resolveRepoPath helper in moduleHelpers.ts alongside propString and propBoolean. Update imgDesktopUpload.ts (27-27), imgRepositoryUpload.ts (27-27), and videoBrowseRepo.ts (27-27) to use the helper with the existing '/static-assets/' fallback.studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/sharedContent.ts (1)
85-103: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse
expandPathOrRawinstead of reimplementing path expansion.The
run()function manually chainsctx.expandPath?.(...) ?? options.target.path ?? repoPath. TheexpandPathOrRawhelper (already used insimpleTaxonomy.tsand referenced frompathUtils.ts) does the same thing. Import it here for consistency with the rest of the data-source modules.♻️ Proposed refactor
-import { - capabilitiesFromActions, - createBrowseAction, - createCreateAction, - createSearchAction, - type CreateTarget, - propBoolean, - propString -} from '../moduleHelpers'; +import { + capabilitiesFromActions, + createBrowseAction, + createCreateAction, + createSearchAction, + type CreateTarget, + propBoolean, + propString +} from '../moduleHelpers'; +import { expandPathOrRaw } from '../pathUtils';return ctx.services.createContent({ - path: ctx.expandPath?.(options.target.path ?? repoPath) ?? options.target.path ?? repoPath, + path: expandPathOrRaw(ctx, options.target.path ?? repoPath), contentTypeId: options.target.contentTypeId, embedded: options.target.strategy === 'embedded' });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/sharedContent.ts` around lines 85 - 103, Update the create action’s run method to import and use the existing expandPathOrRaw helper instead of manually chaining ctx.expandPath, options.target.path, and repoPath. Preserve the current path precedence and pass the resolved path to ctx.services.createContent.studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/keyValueList.ts (1)
22-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated key/value normalization logic into
moduleHelpers.ts.keyValueList.ts'sparseKeyValueOptionsandsimpleTaxonomy.ts'sitemsFromTaxonomyDocboth coercekey/valueto strings, throw when both are empty, and spread the remaining properties — the shared root cause is that thisDataSourceListItemnormalization step has not been factored into a common helper.
studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/keyValueList.ts#L33-L38: replace the inline per-item normalization with a shared helper, e.g.normalizeListItem(item, index, sourceLabel).studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/simpleTaxonomy.ts#L38-L43: replace the inline per-item normalization with the same shared helper.♻️ Proposed helper addition
// in moduleHelpers.ts +export function normalizeListItem(item: unknown, index: number, sourceLabel: string): DataSourceListItem { + if (!item || typeof item !== 'object') { + throw new Error(`${sourceLabel} entry at index ${index} is not an object.`); + } + const record = item as Record<string, unknown>; + const key = String(record.key ?? ''); + const value = String(record.value ?? ''); + if (!key && !value) { + throw new Error(`${sourceLabel} entry at index ${index} is missing key/value.`); + } + return { ...record, key, value }; +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/keyValueList.ts` around lines 22 - 40, Extract the duplicated DataSourceListItem normalization from parseKeyValueOptions into a shared normalizeListItem helper in moduleHelpers.ts, preserving string coercion, empty key/value validation, property spreading, and indexed error messages. Update keyValueList.ts lines 22-40 and simpleTaxonomy.ts lines 26-45 to use the helper with the appropriate source label; both sites require direct changes.studio-ui/ui/app/src/components/FormsEngine/controls/ImagePicker.tsx (1)
229-235: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an empty state when data sources resolve with no actions. All three pickers compute
actionsReadyasBoolean(context) && actions.length > 0 && !loading. When the status isreadybut the field has no bound actions, the final branch renders an action container with no children and no message, so the author sees a blank area.NodeSelectorhandles the same condition withEmptyState.
studio-ui/ui/app/src/components/FormsEngine/controls/ImagePicker.tsx#L229-L235: add a branch before the action box that rendersEmptyStatewhen!actionsReadyand the status is notloadingorerror.studio-ui/ui/app/src/components/FormsEngine/controls/VideoPicker.tsx#L177-L186: add the same branch before theBoxthat rendersmenuOptions.studio-ui/ui/app/src/components/FormsEngine/controls/TranscodedVideoPicker.tsx#L140-L149: add the same branch before theBoxthat rendersactionMenuItems.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio-ui/ui/app/src/components/FormsEngine/controls/ImagePicker.tsx` around lines 229 - 235, Add an EmptyState branch before the action container in ImagePicker.tsx (lines 229-235), VideoPicker.tsx (lines 177-186), and TranscodedVideoPicker.tsx (lines 140-149). Render it when actionsReady is false while the corresponding data-source status is neither loading nor error, so ready fields with no bound actions show an empty-state message instead of a blank container.studio-ui/ui/app/src/components/FormsEngine/components/GroupedDataSourceActionMenuItems.tsx (1)
152-180: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftForward MUI focus props to a direct
MenuItem.
MenuListseesGroupedDataSourceActionMenuItemsas one child. It injectsautoFocusandtabIndex={0}into that child, but this component drops both props. ItsMenuItemelements therefore keeptabIndex={-1}, so the host menu cannot assign initial focus or keyboard navigation correctly.Lift the item rendering into the host menu, or forward these props to the intended
MenuItem. The nestedMenuandDialogare not directMenuListchildren.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio-ui/ui/app/src/components/FormsEngine/components/GroupedDataSourceActionMenuItems.tsx` around lines 152 - 180, Update GroupedDataSourceActionMenuItems to accept and forward the MUI autoFocus and tabIndex props from MenuList to the intended direct MenuItem, preserving the existing choice rendering and handlers. Do not apply these props to the nested Menu or Dialog; ensure the host menu can assign initial focus and keyboard navigation.studio-ui/.cursor/rules/type-builder-forms-engine.mdc (1)
22-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGeneralize the hardcoded personal path.
Line 26 hardcodes an absolute path under a specific user's home directory (
/Users/rart/Workspace/...), and lines 32-33 hardcode sandbox subpaths under it. This rules file guides any contributor or AI agent doing Type Builder / Forms Engine work, but the path only resolves on one developer's machine. Replace it with a placeholder that each contributor can set locally, or move the value out of the shared rules file.📝 Proposed generalization
-The CrafterCMS authoring instance used for this work is: - -`/Users/rart/Workspace/craftercms/develop-2026.07.29.11.28` - -Site sandboxes (source of truth for form-definitions / site config — **not** the sample XML under `ui/app/src/components/ContentTypeManagement/` or `samples/`): - -| Site | Path | -|------|------| -| editorial | `/Users/rart/Workspace/craftercms/develop-2026.07.29.11.28/crafter-authoring/data/repos/sites/editorial/sandbox` | -| craftercms | `/Users/rart/Workspace/craftercms/develop-2026.07.29.11.28/crafter-authoring/data/repos/sites/craftercmscom/sandbox` | +The CrafterCMS authoring instance used for this work is `<AUTHORING_INSTANCE_ROOT>` (set this per developer machine). + +Site sandboxes (source of truth for form-definitions / site config — **not** the sample XML under `ui/app/src/components/ContentTypeManagement/` or `samples/`): + +| Site | Path | +|------|------| +| editorial | `<AUTHORING_INSTANCE_ROOT>/crafter-authoring/data/repos/sites/editorial/sandbox` | +| craftercms | `<AUTHORING_INSTANCE_ROOT>/crafter-authoring/data/repos/sites/craftercmscom/sandbox` |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio-ui/.cursor/rules/type-builder-forms-engine.mdc` around lines 22 - 34, Generalize the hardcoded authoring-instance path in the “Runtime environment” section by replacing the user-specific absolute path and derived sandbox paths with a contributor-configurable placeholder or external local configuration value. Preserve the editorial and craftercms site entries while ensuring the shared rules file no longer depends on `/Users/rart` or any other machine-specific directory.studio-ui/docs/type-builder-forms-engine-plugins.md (1)
33-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd language labels to fenced code blocks.
Add
textto these structural and pseudocode blocks. This fixes the reported markdownlint warnings.Also applies to: 172-183, 481-485, 491-495
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio-ui/docs/type-builder-forms-engine-plugins.md` around lines 33 - 39, Add the text language label to the fenced code blocks containing the plugin source and installed site path examples, as well as the additional blocks at the referenced sections. Preserve their existing structural and pseudocode content while ensuring every affected fence uses the labeled form to satisfy markdownlint.Source: Linters/SAST tools
studio-ui/docs/type-builder-forms-engine.md (1)
20-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd language labels to fenced code blocks.
Add
textto these structural diagrams. This fixes the reported markdownlint warnings.Also applies to: 72-88, 94-105
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio-ui/docs/type-builder-forms-engine.md` around lines 20 - 29, Add the `text` language label to the fenced code blocks containing the structural diagrams in this document, including the sections around the referenced ranges, while leaving the diagram content unchanged.Source: Linters/SAST tools
studio-ui/ui/app/src/components/FormsEngine/dataSources/actionAdapters.ts (1)
44-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLocalize the group labels.
actionKindLabelderives a user-visible label by capitalizing the raw kind id, so the menu always shows English text. Map the built-in kinds toreact-intlmessages and fall back to the action label for custom kinds.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio-ui/ui/app/src/components/FormsEngine/dataSources/actionAdapters.ts` around lines 44 - 46, Update actionKindLabel to resolve built-in DataSourceActionKind values through the existing react-intl message definitions, returning the localized label for the current locale. Preserve a fallback to the existing capitalization logic for custom or unmapped kinds, and use the appropriate intl access available in actionAdapters.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@studio-ui/docs/type-builder-forms-engine.md`:
- Around line 391-407: Update studio-ui/docs/type-builder-forms-engine.md:
revise Section 5.8 to describe FE control and data-source plugins using
PluginDescriptor.controls and PluginDescriptor.dataSources, removing the
obsolete default-React-component and no-executable-data-source claims; update
the document’s last-updated date at line 5 to 2026-07-31; and incorporate the
durable findings into the Progress and Open decisions sections.
In
`@studio-ui/ui/app/src/components/ContentTypeManagement/components/EditTypeView.tsx`:
- Around line 579-592: Mark both insertion flows as pending changes by calling
onUpdateHasPendingChanges(true) after the type is updated in handleInsertField
and after the data source is added in the sibling insertion path, preserving the
existing update behavior.
In
`@studio-ui/ui/app/src/components/ContentTypeManagement/descriptors/controls/pageNavOrder.ts`:
- Around line 40-44: Update the page-nav-order descriptor’s metadata and
validation around the page-nav-order control so it is available only as a root
field, excluding it from repeat-group or other nested field insertion paths that
inspect type.fields. Preserve the existing placeInNav and orderDefault_f
identifiers for the root-field configuration.
In `@studio-ui/ui/app/src/components/FormsEngine/controls/DateTime.tsx`:
- Around line 39-46: Update the expiredDateDescriptor metadata to include the
required {id}_tz field used by the DateTime control. Ensure bootstrap creates
this timezone atom so DateTime can initialize without throwing, while preserving
the descriptor’s existing metadata.
In `@studio-ui/ui/app/src/components/FormsEngine/controls/ImagePicker.tsx`:
- Around line 96-109: Update the promise chain in the ImagePicker change handler
around validateImageRestrictions so rejected inspections are handled with a
catch branch. Provide user feedback through the existing image-picker error
mechanism and preserve the current setValue/crop-dialog behavior for successful
validation.
In `@studio-ui/ui/app/src/components/FormsEngine/controls/PageNavOrder.tsx`:
- Around line 220-222: Remove the development draft Alert from the PageNavOrder
control so users are not shown an unimplemented-feature notice. If an interim
message must remain, replace the raw text with a localized FormattedMessage
consistent with the other labels in PageNavOrder.
In `@studio-ui/ui/app/src/components/FormsEngine/controls/VideoPicker.tsx`:
- Around line 126-134: Update the dimension display branch in VideoPicker’s
render to safely access videoInfo.width and videoInfo.height when videoInfo may
be nullish, matching ImagePicker’s optional-access pattern. Preserve the
existing loading and error branches and ensure rendering does not throw when
dimensions are unavailable.
In
`@studio-ui/ui/app/src/components/FormsEngine/dataSourceHooks/useDataSourceListOptions.ts`:
- Around line 57-86: Update the early-return branch in the useEffect of
useDataSourceListOptions so that when status is no longer 'loading' and context
is absent, it sets loaded for the current key with an empty groups list before
returning. Preserve the existing cleanup behavior and continue the normal list
resolution path when context is available.
In `@studio-ui/ui/app/src/components/FormsEngine/dataSources/moduleHelpers.ts`:
- Around line 155-168: Update the upload-result handling branch in the candidate
mapping function to reject results when both meta.path and meta.name are absent,
rather than returning an asset with an empty relativeUrl. Throw the existing
mapping error used for invalid candidates, while preserving the current URL
construction when either value is available.
In `@studio-ui/ui/app/src/components/FormsEngine/dataSources/types.ts`:
- Around line 24-28: Update the DataSourceCustomSelection type to use a fixed
kind discriminant of 'custom' and add a separate customKind: string field,
preventing overlap with built-in selection kinds. Keep DataSourceInterface and
DataSourceActionKind open-ended, using string & {} only where autocomplete is
required, and ensure DataSourceSelection can narrow reliably on kind checks.
In `@studio-ui/ui/app/src/components/FormsEngine/FormsEngine.tsx`:
- Around line 392-423: Clone repeat.values when initializing values in the
FormsEngine repeat-field flow, so subsequent additions of missing
additionalFieldIds mutate only a local working copy. Preserve the existing
createParsedValuesObject path when repeat.values is absent and keep
atomValueCreator processing unchanged.
In `@studio-ui/ui/app/src/components/FormsEngine/lib/controlHelpers.tsx`:
- Around line 82-94: Update useControlPluginModule and the controlPluginCache
flow so the complete promise returned by loadControlPluginModule is cached,
keyed by plugin URL and control type. Reuse that cached final promise across
renders instead of creating a new loading.then(...) promise on each call,
ensuring PluginControlRenderer receives stable async state.
In `@studio-ui/ui/app/src/components/FormsEngine/lib/controlPluginLoader.ts`:
- Around line 111-118: Update useControlPluginModule and the
loadControlPluginModule flow to cache the final Promise<LoadedControlPlugin> by
plugin URL and controlType before passing it to use(). Reuse the cached promise
for both the Promise.resolve fast path and loading.then path, ensuring repeated
renders do not create uncached promises.
In `@studio-ui/ui/app/src/components/FormsEngine/lib/rteUtils.ts`:
- Around line 151-164: Update the media selection callback around the
actions/dataSources.context guard to handle the empty-actions or missing-context
branch by notifying the author and invoking cb with the appropriate empty
result, following the existing noDatasourcesConfigured notification pattern in
paste_postprocess. Preserve the current invokeActionChoice and
openDataSourcePicker behavior when a valid media data source is available.
In `@studio-ui/ui/app/src/components/FormsEngine/lib/valueRetrievers.ts`:
- Around line 123-134: Update the additional-field handling in the content-type
iteration to parse each field using its own value type rather than the parent
field’s type. In particular, ensure orderDefault_f uses numberFieldExtractor so
numeric values such as -1 and 9000 remain numbers before assigning values and
invoking fieldCallback.
In `@studio-ui/ui/app/src/hooks/useVideoInfo.ts`:
- Around line 82-99: In the metadata fetch success path inside the asynchronous
flow, check abortController.signal.aborted immediately before applying results
with setVideoInfo and setIsFetchingMetadata. Return without updating state when
the request was cancelled, matching the existing cancellation handling in the
catch branch and the dimensions flow.
In `@studio-ui/ui/app/src/services/plugin.ts`:
- Around line 208-211: Update the plugin control registration flow around
registerControlContribution and registerControlDataSourceBindings to prevent
plugin typeKey values from overriding built-in control types. Either reject
collisions with the built-in control registry before registering plugin
contributions, or ensure getControlDataSourceBindings always resolves built-in
bindings before runtime plugin registrations.
In `@studio-ui/ui/app/src/state/reducers/uiConfig.ts`:
- Around line 110-134: Update the ContentTypeManagement configuration parsing in
the controls iteration to convert the deserialized descriptor value with asArray
and process each descriptor independently. Use each descriptor’s own id for both
the controls key and the normalized id field, while preserving the existing
fields, sections, and metadata normalization for each entry; do not use the
container tag.id.
- Line 119: Update the fields assignment in the descriptor mapping to explicitly
detect the empty-array result from deserializing <fields /> and replace it with
an empty object map; preserve valid object-shaped descriptor.fields values
unchanged.
---
Outside diff comments:
In `@studio-ui/ui/app/src/components/FormsEngine/FormsEngine.tsx`:
- Around line 605-621: Add customControls to the dependency array of the
surrounding FormsEngine effect so it reruns when state.uiConfig.controls is
replaced after mount. Preserve the existing dependencies and use the
customControls value already derived in the component; do not rely on
importPlugin or registerPlugin registry updates.
---
Nitpick comments:
In `@studio-ui/.cursor/rules/type-builder-forms-engine.mdc`:
- Around line 22-34: Generalize the hardcoded authoring-instance path in the
“Runtime environment” section by replacing the user-specific absolute path and
derived sandbox paths with a contributor-configurable placeholder or external
local configuration value. Preserve the editorial and craftercms site entries
while ensuring the shared rules file no longer depends on `/Users/rart` or any
other machine-specific directory.
In `@studio-ui/docs/type-builder-forms-engine-plugins.md`:
- Around line 33-39: Add the text language label to the fenced code blocks
containing the plugin source and installed site path examples, as well as the
additional blocks at the referenced sections. Preserve their existing structural
and pseudocode content while ensuring every affected fence uses the labeled form
to satisfy markdownlint.
In `@studio-ui/docs/type-builder-forms-engine.md`:
- Around line 20-29: Add the `text` language label to the fenced code blocks
containing the structural diagrams in this document, including the sections
around the referenced ranges, while leaving the diagram content unchanged.
In
`@studio-ui/ui/app/src/components/FormsEngine/components/GroupedDataSourceActionMenuItems.tsx`:
- Around line 152-180: Update GroupedDataSourceActionMenuItems to accept and
forward the MUI autoFocus and tabIndex props from MenuList to the intended
direct MenuItem, preserving the existing choice rendering and handlers. Do not
apply these props to the nested Menu or Dialog; ensure the host menu can assign
initial focus and keyboard navigation.
In `@studio-ui/ui/app/src/components/FormsEngine/controls/ImagePicker.tsx`:
- Around line 229-235: Add an EmptyState branch before the action container in
ImagePicker.tsx (lines 229-235), VideoPicker.tsx (lines 177-186), and
TranscodedVideoPicker.tsx (lines 140-149). Render it when actionsReady is false
while the corresponding data-source status is neither loading nor error, so
ready fields with no bound actions show an empty-state message instead of a
blank container.
In `@studio-ui/ui/app/src/components/FormsEngine/dataSources/actionAdapters.ts`:
- Around line 44-46: Update actionKindLabel to resolve built-in
DataSourceActionKind values through the existing react-intl message definitions,
returning the localized label for the current locale. Preserve a fallback to the
existing capitalization logic for custom or unmapped kinds, and use the
appropriate intl access available in actionAdapters.ts.
In
`@studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/audioBrowseRepo.ts`:
- Around line 26-41: Extract the shared repo-path fallback into resolveRepoPath
in moduleHelpers.ts, preserving repoPath, path, and the '/static-assets/'
default precedence. Replace the inline fallback in create in
studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/audioBrowseRepo.ts
lines 26-41, audioDesktopUpload.ts lines 26-41, and videoDesktopUpload.ts lines
26-41 with resolveRepoPath(record, '/static-assets/').
In
`@studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/imgDesktopUpload.ts`:
- Line 27: Extract the repeated repository-path fallback into a shared
resolveRepoPath helper in moduleHelpers.ts alongside propString and propBoolean.
Update imgDesktopUpload.ts (27-27), imgRepositoryUpload.ts (27-27), and
videoBrowseRepo.ts (27-27) to use the helper with the existing '/static-assets/'
fallback.
In
`@studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/keyValueList.ts`:
- Around line 22-40: Extract the duplicated DataSourceListItem normalization
from parseKeyValueOptions into a shared normalizeListItem helper in
moduleHelpers.ts, preserving string coercion, empty key/value validation,
property spreading, and indexed error messages. Update keyValueList.ts lines
22-40 and simpleTaxonomy.ts lines 26-45 to use the helper with the appropriate
source label; both sites require direct changes.
In
`@studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/sharedContent.ts`:
- Around line 85-103: Update the create action’s run method to import and use
the existing expandPathOrRaw helper instead of manually chaining ctx.expandPath,
options.target.path, and repoPath. Preserve the current path precedence and pass
the resolved path to ctx.services.createContent.
In `@studio-ui/ui/app/src/components/FormsEngine/lib/valueRetrievers.ts`:
- Around line 121-122: Extract the duplicated customControls/controlDescriptors
merge into a shared resolveControlDescriptors helper, preserving the current
merge order. In
studio-ui/ui/app/src/components/FormsEngine/lib/valueRetrievers.ts lines
121-122, replace the inline merge and TODO with the helper call; make the
corresponding replacement in
studio-ui/ui/app/src/components/FormsEngine/lib/formUtils.tsx lines 880-897,
importing or defining the shared helper as appropriate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1188c527-b5a4-4f9f-98f3-9c2dc839607b
📒 Files selected for processing (93)
studio-ui/.cursor/rules/type-builder-forms-engine.mdcstudio-ui/AGENTS.mdstudio-ui/docs/fe2-control-plugin-single-model-implementation.mdstudio-ui/docs/type-builder-forms-engine-plugins.mdstudio-ui/docs/type-builder-forms-engine.mdstudio-ui/ui/app/src/components/ContentTypeManagement/components/AdditionalFieldChip.tsxstudio-ui/ui/app/src/components/ContentTypeManagement/components/EditTypeView.tsxstudio-ui/ui/app/src/components/ContentTypeManagement/components/FieldChip.tsxstudio-ui/ui/app/src/components/ContentTypeManagement/descriptors/controls/dateTime.tsstudio-ui/ui/app/src/components/ContentTypeManagement/descriptors/controls/pageNavOrder.tsstudio-ui/ui/app/src/components/ContentTypeManagement/utils.tsstudio-ui/ui/app/src/components/DateTimeTimezonePicker/DateTimeTimezonePicker.tsxstudio-ui/ui/app/src/components/FormsEngine/FormsEngine.tsxstudio-ui/ui/app/src/components/FormsEngine/components/GroupedDataSourceActionMenuItems.tsxstudio-ui/ui/app/src/components/FormsEngine/components/SortableList.tsxstudio-ui/ui/app/src/components/FormsEngine/controls/CheckboxGroup.tsxstudio-ui/ui/app/src/components/FormsEngine/controls/DateTime.tsxstudio-ui/ui/app/src/components/FormsEngine/controls/Dropdown.tsxstudio-ui/ui/app/src/components/FormsEngine/controls/ImagePicker.tsxstudio-ui/ui/app/src/components/FormsEngine/controls/NodeSelector.tsxstudio-ui/ui/app/src/components/FormsEngine/controls/PageNavOrder.tsxstudio-ui/ui/app/src/components/FormsEngine/controls/RichTextEditor.tsxstudio-ui/ui/app/src/components/FormsEngine/controls/TranscodedVideoPicker.tsxstudio-ui/ui/app/src/components/FormsEngine/controls/VideoPicker.tsxstudio-ui/ui/app/src/components/FormsEngine/controls/registry.tsstudio-ui/ui/app/src/components/FormsEngine/dataSourceHooks/useConsolidatedImagePickerData.tsstudio-ui/ui/app/src/components/FormsEngine/dataSourceHooks/useConsolidatedItemPickerData.tsstudio-ui/ui/app/src/components/FormsEngine/dataSourceHooks/useDataSourceListOptions.tsstudio-ui/ui/app/src/components/FormsEngine/dataSourceHooks/useExtractDataSources.tsxstudio-ui/ui/app/src/components/FormsEngine/dataSources/actionAdapters.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/bindings.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/completeness.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/defineModule.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/host.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/index.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/loader.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/moduleHelpers.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/audioBrowseRepo.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/audioDesktopUpload.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/components.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/componentsPagesShared.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/configuredList.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/embeddedContent.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/fileBrowseRepo.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/fileDesktopUpload.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/flashDesktopUpload.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/imgDesktopUpload.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/imgRepositoryUpload.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/imgS3Repo.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/imgS3Upload.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/imgWebDAVRepo.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/imgWebDAVUpload.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/index.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/keyValueList.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/pages.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/remoteStubs.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/s3Repo.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/s3Upload.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/sharedContent.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/simpleTaxonomy.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/videoBrowseRepo.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/videoDesktopUpload.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/videoS3Repo.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/videoS3Transcoding.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/videoS3Upload.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/videoWebDAVRepo.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/videoWebDAVUpload.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/webDavRepo.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/webDavUpload.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/pathUtils.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/registry.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/services.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/types.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/useFieldDataSources.tsstudio-ui/ui/app/src/components/FormsEngine/index.tsstudio-ui/ui/app/src/components/FormsEngine/lib/controlHelpers.tsxstudio-ui/ui/app/src/components/FormsEngine/lib/controlMap.tsstudio-ui/ui/app/src/components/FormsEngine/lib/controlPluginLoader.tsstudio-ui/ui/app/src/components/FormsEngine/lib/dataSourceMap.tsstudio-ui/ui/app/src/components/FormsEngine/lib/formUtils.tsxstudio-ui/ui/app/src/components/FormsEngine/lib/rteUtils.tsstudio-ui/ui/app/src/components/FormsEngine/lib/valueRetrievers.tsstudio-ui/ui/app/src/components/FormsEngine/lib/valueSerializers.tsstudio-ui/ui/app/src/components/FormsEngine/types.tsstudio-ui/ui/app/src/env/craftercms.tsstudio-ui/ui/app/src/hooks/useVideoInfo.tsstudio-ui/ui/app/src/models/ContentType.tsstudio-ui/ui/app/src/models/GlobalState.tsstudio-ui/ui/app/src/models/PluginDescriptor.tsstudio-ui/ui/app/src/services/content.tsstudio-ui/ui/app/src/services/contentTypes.tsstudio-ui/ui/app/src/services/plugin.tsstudio-ui/ui/app/src/state/reducers/uiConfig.ts
💤 Files with no reviewable changes (3)
- studio-ui/ui/app/src/components/FormsEngine/lib/dataSourceMap.ts
- studio-ui/ui/app/src/components/FormsEngine/dataSourceHooks/useConsolidatedItemPickerData.ts
- studio-ui/ui/app/src/components/FormsEngine/dataSourceHooks/useConsolidatedImagePickerData.ts
Contributor
Author
|
@coderabbitai FE2 implemented data sources in a way that didn't support plugins and the implementation was coupled to the existing data sources — code would be to be written to incorporate new DS. There was no shared, durable map of how Type Builder and Forms Engine load controls, data sources, plugins, and form controllers—especially what legacy did well versus what the new React stack still couldn’t do dynamically. See docs on this PR (i.e. studio-ui/.cursor/rules/type-builder-forms-engine.mdc, studio-ui/docs/*) to understand how we framed and solved the problems. Based on this changes on this PR, did we solve these issues? Did we do it with the right architecture? Does the code successfully implement the design? Do you see any issues? What comments do you have? Perform a comprehensive architectural review and report. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== TARGET FILES ==='
fd -t f . studio-ui | rg '(\.cursor/rules/type-builder-forms-engine\.mdc|docs/(type-builder-forms-engine|fe2-control-plugin-single-model-implementation)|ui/app/src/(services/plugin|models/PluginDescriptor|components/FormsEngine/(dataSources/(loader|registry|bindings|useFieldDataSources|index)|lib/(controlHelpers|controlPluginLoader)|controls/registry)|env/craftercms)\.ts)'
echo '=== OUTLINES ==='
for f in \
studio-ui/ui/app/src/services/plugin.ts \
studio-ui/ui/app/src/components/FormsEngine/dataSources/loader.ts \
studio-ui/ui/app/src/components/FormsEngine/dataSources/useFieldDataSources.ts \
studio-ui/ui/app/src/components/FormsEngine/lib/controlPluginLoader.ts \
studio-ui/ui/app/src/components/FormsEngine/lib/controlHelpers.tsx \
studio-ui/ui/app/src/components/FormsEngine/dataSources/bindings.ts \
studio-ui/ui/app/src/components/FormsEngine/controls/registry.ts \
studio-ui/ui/app/src/models/PluginDescriptor.ts; do
if [ -f "$f" ]; then
echo "--- $f ---"
ast-grep outline "$f" --items all --view compact || true
fi
done
echo '=== DESIGN KEYWORDS ==='
rg -n -C 2 --glob '*.md' --glob '*.mdc' \
'PluginDescriptor|importPlugin|registerPlugin|control|data.source|form.controller|dynamic|legacy|non.goal|acceptance' \
studio-ui/.cursor/rules/type-builder-forms-engine.mdc studio-ui/docs 2>/dev/null | head -n 500Length of output: 63110 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== SOURCE OUTLINES ==='
for f in \
studio-ui/ui/app/src/services/plugin.ts \
studio-ui/ui/app/src/components/FormsEngine/dataSources/loader.ts \
studio-ui/ui/app/src/components/FormsEngine/dataSources/useFieldDataSources.ts \
studio-ui/ui/app/src/components/FormsEngine/lib/controlPluginLoader.ts \
studio-ui/ui/app/src/components/FormsEngine/lib/controlHelpers.tsx \
studio-ui/ui/app/src/components/FormsEngine/dataSources/bindings.ts \
studio-ui/ui/app/src/components/FormsEngine/controls/registry.ts \
studio-ui/ui/app/src/components/FormsEngine/dataSources/registry.ts \
studio-ui/ui/app/src/models/PluginDescriptor.ts \
studio-ui/ui/app/src/components/ContentTypeManagement/components/EditTypeView.tsx \
studio-ui/ui/app/src/components/ContentTypeManagement/utils.ts; do
echo "--- $f ---"
ast-grep outline "$f" --items all --view signatures || true
done
echo '=== RUNTIME IMPLEMENTATIONS ==='
for f in \
studio-ui/ui/app/src/services/plugin.ts \
studio-ui/ui/app/src/components/FormsEngine/dataSources/loader.ts \
studio-ui/ui/app/src/components/FormsEngine/dataSources/useFieldDataSources.ts \
studio-ui/ui/app/src/components/FormsEngine/lib/controlPluginLoader.ts \
studio-ui/ui/app/src/components/FormsEngine/lib/controlHelpers.tsx \
studio-ui/ui/app/src/components/FormsEngine/dataSources/bindings.ts \
studio-ui/ui/app/src/components/FormsEngine/controls/registry.ts \
studio-ui/ui/app/src/components/FormsEngine/dataSources/registry.ts \
studio-ui/ui/app/src/models/PluginDescriptor.ts; do
echo "--- $f ---"
cat -n "$f"
done
echo '=== INTEGRATION CALL SITES ==='
rg -n -C 4 --type ts --type tsx \
'\b(importPlugin|registerPlugin|loadDataSourceModule|useFieldDataSources|useControlPluginModule|registerControlDataSourceBindings|extractPluginLocatorFromConfigEntry|plugin:\s*|hasJsController)\b' \
studio-ui/ui/app/src | head -n 1200Length of output: 104596 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== LOCATOR MODEL, TYPE BUILDER, AND HOSTS ==='
for spec in \
'studio-ui/ui/app/src/models/ContentType.ts:1:240' \
'studio-ui/ui/app/src/components/ContentTypeManagement/components/EditTypeView.tsx:1230:1365' \
'studio-ui/ui/app/src/components/ContentTypeManagement/utils.ts:570:750' \
'studio-ui/ui/app/src/components/FormsEngine/dataSources/host.ts:1:180' \
'studio-ui/ui/app/src/env/craftercms.ts:1:180' \
'studio-ui/samples/fe2-control-plugin.example.mjs:1:260'; do
file="${spec%%:*}"
rest="${spec#*:}"; start="${rest%%:*}"; end="${rest##*:}"
if [ -f "$file" ]; then
echo "--- $file lines $start-$end ---"
sed -n "${start},${end}p" "$file" | nl -ba -v "$start"
fi
done
echo '=== PLUGIN IDENTITY AND LOCATOR DOCUMENTATION ==='
rg -n -C 4 --glob '*.md' --glob '*.mdc' \
'pluginId|PluginDescriptor\.id|runtime.*identity|asset locator|identity|filename|file.*filename' \
studio-ui/docs studio-ui/.cursor/rules/type-builder-forms-engine.mdc | head -n 700
echo '=== TESTS AND SPECIMENS ==='
fd -t f . studio-ui | rg -i '(test|spec|sample).*?(plugin|datasource|forms|control)|(plugin|datasource|forms|control).*?(test|spec|sample)' | head -n 500
rg -n -C 3 \
'(loadControlPluginModule|loadDataSourceModule|registerPlugin\(|registerControlContribution|resolveFieldDataSources|FormDefinitionPlugin|pluginId)' \
studio-ui --glob '*.{test,spec}.{ts,tsx,js,jsx}' --glob '*.mjs' 2>/dev/null | head -n 1000
echo '=== FORM CONTROLLER IMPLEMENTATION CHECK ==='
rg -n -C 3 \
'(hasJsController|form-controller|onBeforeSave|isFieldRelevant|initialize\(form)' \
studio-ui/ui/app/src/components/FormsEngine studio-ui/ui/app/src/models/ContentType.ts | head -n 500Length of output: 2183 🏁 Script executed: #!/bin/bash
set -euo pipefail
print_range () {
file="$1"; start="$2"; end="$3"
if [ -f "$file" ]; then
echo "--- $file lines $start-$end ---"
sed -n "${start},${end}{=;p;}" "$file"
fi
}
echo '=== LOCATOR MODEL, TYPE BUILDER, AND HOSTS ==='
print_range studio-ui/ui/app/src/models/ContentType.ts 1 240
print_range studio-ui/ui/app/src/components/ContentTypeManagement/components/EditTypeView.tsx 1230 1365
print_range studio-ui/ui/app/src/components/ContentTypeManagement/utils.ts 570 750
print_range studio-ui/ui/app/src/components/FormsEngine/dataSources/host.ts 1 180
print_range studio-ui/ui/app/src/env/craftercms.ts 1 180
print_range studio-ui/samples/fe2-control-plugin.example.mjs 1 260
echo '=== PLUGIN IDENTITY AND LOCATOR DOCUMENTATION ==='
rg -n -C 4 --glob '*.md' --glob '*.mdc' \
'pluginId|PluginDescriptor\.id|runtime.*identity|asset locator|identity|filename|file.*filename' \
studio-ui/docs studio-ui/.cursor/rules/type-builder-forms-engine.mdc | head -n 700 || true
echo '=== TEST AND SAMPLE FILES ==='
fd -t f . studio-ui | rg -i '(test|spec|sample).*?(plugin|datasource|forms|control)|(plugin|datasource|forms|control).*?(test|spec|sample)' | head -n 500 || true
echo '=== TEST REFERENCES ==='
rg -n -C 3 \
'(loadControlPluginModule|loadDataSourceModule|registerPlugin\(|registerControlContribution|resolveFieldDataSources|FormDefinitionPlugin|pluginId)' \
studio-ui -g '*test*' -g '*spec*' -g '*.mjs' | head -n 1000 || true
echo '=== FORM CONTROLLER IMPLEMENTATION CHECK ==='
rg -n -C 3 \
'(hasJsController|form-controller|onBeforeSave|isFieldRelevant|initialize\(form)' \
studio-ui/ui/app/src/components/FormsEngine studio-ui/ui/app/src/models/ContentType.ts | head -n 500 || trueLength of output: 54288
Overall assessmentArchitecture: sound, with two important runtime defects. The implementation now has a clear runtime model:
This is the right direction. It replaces central data-source-type branching with typed modules, instance creation, interface checks, capability checks, owner-bound actions, and error isolation. The code also implements the documented single descriptor path for both plugin kinds:
The documentation is also useful and unusually complete. It distinguishes legacy behavior, current FE2 behavior, completed work, and planned work. Important issue: control plugins compare two unrelated identitiesSeverity: high.
After a plugin loads, Lines 95-101 compare that value with These values have different meanings:
The documentation explicitly states that package identity, asset locator identity, and As a result, a valid control plugin can load, register its control contribution, and then be rejected because its descriptor ID does not equal the asset locator ID. The loader should validate ownership against the descriptor returned by the current if (contribution.pluginId !== descriptor.id) {
// reject
}The fast path at Lines 66-69 cannot safely compare a locator ID with a descriptor ID. It should either:
The registry should also treat a second contribution for the same control type from a different descriptor ID as a conflict. The current same-component exception permits a different plugin to overwrite binding metadata while the control registry retains the first plugin owner. Important issue: plugin registration is not atomicSeverity: medium.
If control registration fails after data-source registration succeeds, the data-source modules remain in the process-wide registry. The descriptor itself is not registered. A retry can therefore execute against partial global state. The control path can also register some controls before a later control contribution fails. This behavior is risky because registry state is global and has no unregister operation. Use a two-phase registration process:
Alternatively, add rollback support to the registries and roll back all contributions when a later registration step fails. What this PR completesThe PR completes these design goals:
The design is a meaningful improvement over the previous FE2 implementation. What remains incomplete by designThe PR documentation correctly identifies these as open work. They should not be described as solved capabilities:
These limitations do not invalidate the completed data-source and control-plugin work. They define the boundary of this PR. Verification and test coverageI found no focused test or spec references for:
The two issues above need targeted tests before this architecture is treated as stable. At minimum, add tests for:
ConclusionThe PR does solve the main extensibility and coupling issue for FE2 data sources. The shared However, the control-plugin identity comparison currently contradicts the documented identity model and can prevent valid control plugins from rendering. The registration process also needs atomicity before broad plugin adoption. The work should be considered a strong architectural foundation, not the completion of all Forms Engine modernization work. ✏️ Learnings added
✅ Action performedReview finished.
|
Contributor
Author
|
@coderabbitai, perform a review of coverage of the migration of the legacy controls & data sources vs the new:
Do you see any gaps or issues? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
legacy_ds='studio-ui/static-assets/components/cstudio-forms/data-sources'
legacy_controls='studio-ui/static-assets/components/cstudio-forms/controls'
fe2_ds='studio-ui/ui/app/src/components/FormsEngine/dataSources/modules'
fe2_controls='studio-ui/ui/app/src/components/FormsEngine/controls'
echo '=== INVENTORY ==='
for dir in "$legacy_ds" "$legacy_controls" "$fe2_ds" "$fe2_controls"; do
echo "--- $dir ---"
if [ -d "$dir" ]; then
fd -t f . "$dir" | sort
else
echo 'DIRECTORY NOT FOUND'
fi
done
echo '=== LEGACY DATA-SOURCE OUTLINES ==='
if [ -d "$legacy_ds" ]; then
find "$legacy_ds" -type f \( -name '*.js' -o -name '*.ts' \) -print0 |
xargs -0 -r -n1 sh -c 'echo "--- $0 ---"; ast-grep outline "$0" --items all --view signatures || true'
fi
echo '=== LEGACY CONTROL OUTLINES ==='
if [ -d "$legacy_controls" ]; then
find "$legacy_controls" -type f \( -name '*.js' -o -name '*.ts' \) -print0 |
xargs -0 -r -n1 sh -c 'echo "--- $0 ---"; ast-grep outline "$0" --items all --view signatures || true'
fi
echo '=== FE2 DATA-SOURCE MODULE OUTLINES AND REGISTRATION ==='
ast-grep outline "$fe2_ds" --items all --view signatures || true
if [ -f "$fe2_ds/index.ts" ]; then
cat -n "$fe2_ds/index.ts"
fi
echo '=== FE2 CONTROL OUTLINES ==='
ast-grep outline "$fe2_controls" --items all --view signatures || true
echo '=== LEGACY TYPE AND CONTROL IDENTIFIERS ==='
rg -n -C 3 --glob '*.{js,ts,tsx,xml}' \
'(dataSourceType|datasourceType|data-source|controlType|fieldType|typeName|register.*[Cc]ontrol|register.*[Dd]ata)' \
"$legacy_ds" "$legacy_controls" 2>/dev/null | head -n 2500 || true
echo '=== FE2 TYPE, CONTROL, AND LEGACY-MIGRATION REFERENCES ==='
rg -n -C 3 --type ts --type tsx \
'(type:\s*['\''"][^'\''"]+|controlMap|controlDataSourceBindings|builtInDataSourceModules|legacy|flash-desktop-upload|linked-dropdown)' \
studio-ui/ui/app/src/components/FormsEngine \
studio-ui/ui/app/src/components/ContentTypeManagement 2>/dev/null | head -n 3500 || true
echo '=== LEGACY FORM-DEFINITION CONFIGURATION REFERENCES ==='
rg -n -C 3 --glob '*.{xml,js,ts,tsx}' \
'(img-repository-upload|img-desktop-upload|file-browse-repo|file-desktop-upload|video-|audio-|shared-content|embedded-content|configured-list|key-value-list|simple-taxonomy|components|pages|linked-dropdown|transcoded-video-picker|page-nav-order)' \
studio-ui/static-assets studio-ui/ui/app/src 2>/dev/null | head -n 3500 || true
echo '=== TEST COVERAGE ==='
rg -n -C 2 --glob '*.{test,spec}.{js,jsx,ts,tsx}' \
'(data[Ss]ource|FormsEngine|Dropdown|CheckboxGroup|ImagePicker|NodeSelector|VideoPicker|DateTime|PageNavOrder)' \
studio-ui 2>/dev/null | head -n 1600 || trueLength of output: 203243 🏁 Script executed: #!/bin/bash
set -euo pipefail
legacy_ds='studio-ui/static-assets/components/cstudio-forms/data-sources'
legacy_controls='studio-ui/static-assets/components/cstudio-forms/controls'
fe2_ds='studio-ui/ui/app/src/components/FormsEngine/dataSources/modules'
fe2_controls='studio-ui/ui/app/src/components/FormsEngine/controls'
python3 - <<'PY'
from pathlib import Path
import re
roots = {
"legacy_ds": Path("studio-ui/static-assets/components/cstudio-forms/data-sources"),
"legacy_controls": Path("studio-ui/static-assets/components/cstudio-forms/controls"),
"fe2_ds": Path("studio-ui/ui/app/src/components/FormsEngine/dataSources/modules"),
"fe2_controls": Path("studio-ui/ui/app/src/components/FormsEngine/controls"),
}
def files(root, extensions):
return sorted(p for p in root.rglob("*") if p.is_file() and p.suffix in extensions)
for key, root in roots.items():
print(f"\n=== {key} FILES ({root}) ===")
ext = {".js"} if key.startswith("legacy") else {".ts", ".tsx"}
for p in files(root, ext):
print(p.as_posix())
print("\n=== LEGACY DATA-SOURCE getName() IDENTIFIERS ===")
for p in files(roots["legacy_ds"], {".js"}):
text = p.read_text(errors="replace")
names = re.findall(r"getName\s*:\s*function\s*\(\s*\)\s*\{\s*return\s*['\"]([^'\"]+)", text)
print(f"{p.name}: {', '.join(names) if names else '-'}")
print("\n=== LEGACY CONTROL REGISTRATION IDENTIFIERS ===")
patterns = [
r"getName\s*:\s*function\s*\(\s*\)\s*\{\s*return\s*['\"]([^'\"]+)",
r"register(?:Control)?\s*\(\s*['\"]([^'\"]+)",
r"moduleLoaded\s*\(\s*['\"]([^'\"]+)",
]
for p in files(roots["legacy_controls"], {".js"}):
text = p.read_text(errors="replace")
values = []
for pattern in patterns:
values.extend(re.findall(pattern, text))
print(f"{p.name}: {', '.join(dict.fromkeys(values)) if values else '-'}")
print("\n=== FE2 DATA-SOURCE MODULE type IDENTIFIERS ===")
for p in files(roots["fe2_ds"], {".ts", ".tsx"}):
text = p.read_text(errors="replace")
types = re.findall(r"\btype\s*:\s*['\"]([^'\"]+)", text)
if types:
print(f"{p.name}: {', '.join(dict.fromkeys(types))}")
print("\n=== FE2 CONTROL MAP IDENTIFIERS ===")
p = Path("studio-ui/ui/app/src/components/FormsEngine/lib/controlMap.ts")
if p.exists():
text = p.read_text(errors="replace")
for key in re.findall(r"^\s*['\"]?([A-Za-z0-9_-]+)['\"]?\s*:\s*(?:lazy|null)", text, re.M):
print(key)
PY
echo '=== DATA-SOURCE BINDINGS ==='
cat -n studio-ui/ui/app/src/components/FormsEngine/dataSources/bindings.ts | sed -n '1,260p'
echo '=== FE2 MODULE REGISTRATION ==='
cat -n "$fe2_ds/index.ts" | sed -n '1,260p'
echo '=== LEGACY AND FE2 REMOTE/STUB IMPLEMENTATIONS ==='
for f in \
"$legacy_ds/img-s3-repo.js" \
"$legacy_ds/img-s3-upload.js" \
"$legacy_ds/img-webdav-repo.js" \
"$legacy_ds/img-webdav-upload.js" \
"$legacy_ds/video-s3-repo.js" \
"$legacy_ds/video-s3-upload.js" \
"$legacy_ds/video-webdav-repo.js" \
"$legacy_ds/video-webdav-upload.js" \
"$legacy_ds/s3-repo.js" \
"$legacy_ds/s3-upload.js" \
"$legacy_ds/webdav-repo.js" \
"$legacy_ds/webdav-upload.js" \
"$fe2_ds/remoteStubs.ts"; do
if [ -f "$f" ]; then
echo "--- $f ---"
rg -n -C 3 '(getName|execute|show|upload|browse|search|unsupported|throw|type:|create:|actions)' "$f" | head -n 180 || true
fi
done
echo '=== LEGACY CONTROL NAMES AND FE2 CONTROL MAP ==='
for f in "$legacy_controls"/*.js; do
[ -f "$f" ] || continue
echo "--- $f ---"
rg -n -C 2 '(getName|register|moduleLoaded|render|create|onChange|dataSource|datasource)' "$f" | head -n 100 || true
done
echo '=== DATA-SOURCE AND CONTROL TEST FILES ==='
fd -t f . studio-ui | rg '(/|^).*\.(test|spec)\.(js|jsx|ts|tsx)$' | rg -i '(forms|datasource|data-source|control|picker|dropdown|checkbox|video|taxonomy)' || trueLength of output: 107805 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== CONTROL MAP AND NULL-CONTROL RUNTIME PATH ==='
sed -n '1,180p' studio-ui/ui/app/src/components/FormsEngine/lib/controlMap.ts | nl -ba
rg -n -C 5 --type ts --type tsx \
'(controlMap\[|UnknownControl|link-input|link-textarea|forcehttps|internal-name)' \
studio-ui/ui/app/src/components/FormsEngine studio-ui/ui/app/src/components/ContentTypeManagement
echo '=== FE2 LINK, TEXT, AND COMPATIBILITY CONTROL IMPLEMENTATIONS ==='
for f in \
studio-ui/ui/app/src/components/FormsEngine/controls/Text.tsx \
studio-ui/ui/app/src/components/FormsEngine/controls/Textarea.tsx \
studio-ui/ui/app/src/components/FormsEngine/controls/AutoFileName.tsx \
studio-ui/ui/app/src/components/FormsEngine/controls/FileName.tsx \
studio-ui/ui/app/src/components/FormsEngine/controls/Checkbox.tsx \
studio-ui/ui/app/src/components/FormsEngine/lib/valueRetrievers.ts \
studio-ui/ui/app/src/components/FormsEngine/lib/valueSerializers.ts \
studio-ui/ui/app/src/components/FormsEngine/lib/validators.ts; do
echo "--- $f ---"
cat -n "$f" | sed -n '1,280p'
done
echo '=== REMOTE DATA-SOURCE ACTION IMPLEMENTATION ==='
for f in \
studio-ui/ui/app/src/components/FormsEngine/dataSources/moduleHelpers.ts \
studio-ui/ui/app/src/components/FormsEngine/dataSources/services.ts \
studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/remoteStubs.ts; do
echo "--- $f ---"
rg -n -C 8 '(createRemoteStubAction|unsupportedRemoteError|remote|browseFiles|search|upload|DataSourceServices)' "$f"
done
echo '=== LEGACY REMOTE DATA-SOURCE OPERATION PATHS ==='
for f in studio-ui/static-assets/components/cstudio-forms/data-sources/{S3,WebDAV,img-S3,img-WebDAV,video-S3,video-WebDAV}-{repo,upload}.js; do
if [ -f "$f" ]; then
echo "--- $f ---"
rg -n -C 5 '(getName|insertImageAction|insertVideoAction|browse|upload|S3|WebDAV|dialog|picker|repository)' "$f" | head -n 260
fi
done
echo '=== RTE FEATURE PARITY ==='
echo '--- legacy RTE plugins ---'
for f in studio-ui/static-assets/components/cstudio-forms/controls/rte-plugins/*.js; do
echo "--- $f ---"
rg -n -C 3 '(PluginManager|add\(|button|menu|command|datasource|image|component|layout|link)' "$f" | head -n 160
done
echo '--- FE2 RTE utility and component ---'
for f in \
studio-ui/ui/app/src/components/FormsEngine/controls/RichTextEditor.tsx \
studio-ui/ui/app/src/components/FormsEngine/lib/rteUtils.ts; do
echo "--- $f ---"
cat -n "$f" | sed -n '1,420p'
done
echo '=== FOCUSED TEST/COMPATIBILITY REFERENCES ==='
rg -n -C 3 --glob '*.{test,spec}.{js,jsx,ts,tsx}' \
'(link-input|link-textarea|S3-repo|WebDAV|remote|rte|date-time|forcehttps|internal-name)' \
studio-ui 2>/dev/null || trueLength of output: 3112 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== CONTROL MAP AND NULL-CONTROL RUNTIME PATH ==='
cat -n studio-ui/ui/app/src/components/FormsEngine/lib/controlMap.ts | sed -n '1,180p'
rg -n -C 5 --type ts --type tsx \
'(controlMap\[|UnknownControl|link-input|link-textarea|forcehttps|internal-name)' \
studio-ui/ui/app/src/components/FormsEngine studio-ui/ui/app/src/components/ContentTypeManagement || true
echo '=== FE2 LINK, TEXT, AND COMPATIBILITY CONTROL IMPLEMENTATIONS ==='
for f in \
studio-ui/ui/app/src/components/FormsEngine/controls/Text.tsx \
studio-ui/ui/app/src/components/FormsEngine/controls/Textarea.tsx \
studio-ui/ui/app/src/components/FormsEngine/controls/AutoFileName.tsx \
studio-ui/ui/app/src/components/FormsEngine/controls/FileName.tsx \
studio-ui/ui/app/src/components/FormsEngine/controls/Checkbox.tsx \
studio-ui/ui/app/src/components/FormsEngine/lib/valueRetrievers.ts \
studio-ui/ui/app/src/components/FormsEngine/lib/valueSerializers.ts \
studio-ui/ui/app/src/components/FormsEngine/lib/validators.ts; do
echo "--- $f ---"
cat -n "$f" | sed -n '1,280p'
done
echo '=== REMOTE DATA-SOURCE ACTION IMPLEMENTATION ==='
for f in \
studio-ui/ui/app/src/components/FormsEngine/dataSources/moduleHelpers.ts \
studio-ui/ui/app/src/components/FormsEngine/dataSources/services.ts \
studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/remoteStubs.ts; do
echo "--- $f ---"
rg -n -C 8 '(createRemoteStubAction|unsupportedRemoteError|remote|browseFiles|search|upload|DataSourceServices)' "$f" || true
done
echo '=== LEGACY REMOTE DATA-SOURCE OPERATION PATHS ==='
for f in studio-ui/static-assets/components/cstudio-forms/data-sources/{S3,WebDAV,img-S3,img-WebDAV,video-S3,video-WebDAV}-{repo,upload}.js; do
if [ -f "$f" ]; then
echo "--- $f ---"
rg -n -C 5 '(getName|insertImageAction|insertVideoAction|browse|upload|S3|WebDAV|dialog|picker|repository)' "$f" | head -n 260 || true
fi
done
echo '=== RTE FEATURE PARITY ==='
echo '--- legacy RTE plugins ---'
for f in studio-ui/static-assets/components/cstudio-forms/controls/rte-plugins/*.js; do
echo "--- $f ---"
rg -n -C 3 '(PluginManager|add\(|button|menu|command|datasource|image|component|layout|link)' "$f" | head -n 160 || true
done
echo '--- FE2 RTE utility and component ---'
for f in \
studio-ui/ui/app/src/components/FormsEngine/controls/RichTextEditor.tsx \
studio-ui/ui/app/src/components/FormsEngine/lib/rteUtils.ts; do
echo "--- $f ---"
cat -n "$f" | sed -n '1,420p'
done
echo '=== FOCUSED TEST/COMPATIBILITY REFERENCES ==='
rg -n -C 3 --glob '*.{test,spec}.{js,jsx,ts,tsx}' \
'(link-input|link-textarea|S3-repo|WebDAV|remote|rte|date-time|forcehttps|internal-name)' \
studio-ui 2>/dev/null || trueLength of output: 150939
Data sourcesCovered by FE2All 27 legacy data-source types have a corresponding FE2 module registration in This includes:
The FE2 module set also adds Functional gap: all remote S3 and WebDAV data sources fail at action execution
However, each module creates its browse or upload action with
This is a functional regression from legacy behavior. The legacy sources call the working platform operations:
The FE2 controls will show these actions because the modules advertise This affects Recommendation: Do not advertise these actions until FE2 services support remote operations, or implement remote browse/upload methods in ControlsComplete control-map identifiers do not mean complete runtime supportThe FE2
Each type has a legacy implementation under The last two are not safe to treat as ordinary text fields. The legacy The FE2 code has value retrievers and serializers for these types. Therefore, existing content can parse and serialize, but Forms Engine cannot render the fields for editing. Recommendation: Either implement the four controls, map them to documented compatible controls, or reject these control types before a form opens with a clear migration error. Leaving them in Controls with adequate migration coverageThe following legacy controls have FE2 replacements:
Rich Text Editor gapThe FE2 RTE now supports action-based selection from image, video, audio, and file data sources. This is a substantial improvement. However, I found no FE2 equivalents for several legacy Crafter-specific RTE plugins:
For example, legacy
This is a migration gap if current RTE configurations reference these legacy features. It needs a per-plugin compatibility decision:
Test coverageI found no focused compatibility tests for:
Add a table-driven test that compares the legacy type list with ConclusionThe migration is strong for local data sources and the main controls. The module architecture correctly removes central type switching. The key gaps are:
These gaps should be explicit release constraints if they are intentionally out of scope. You are interacting with an AI system. |
…ities, and enforce root-only control restrictions.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
studio-ui/ui/app/src/components/ContentTypeManagement/descriptors/controls/expiredDate.ts (1)
142-144: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a descriptor bootstrap regression test.
Verify that an
expiredDatefield with idexpired_dtcreates theexpired_dt_tzatom during normal and repeat/embedded initialization. Verify that the timezone value is preserved through value parsing and form updates.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio-ui/ui/app/src/components/ContentTypeManagement/descriptors/controls/expiredDate.ts` around lines 142 - 144, Add regression coverage for the expiredDate descriptor bootstrap using field id expired_dt: verify normal and repeat/embedded initialization create the expired_dt_tz atom, and verify the timezone value remains intact through value parsing and subsequent form updates.studio-ui/ui/app/src/components/ContentTypeManagement/utils.ts (1)
725-725: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a data-source plugin serialization test.
Test a data source with a plugin locator and one without a plugin. Verify that serialization preserves the locator for plugin-backed data sources and omits it for built-in data sources.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio-ui/ui/app/src/components/ContentTypeManagement/utils.ts` at line 725, Add serialization coverage for the data-source handling around the plugin field, testing both a plugin-backed source with a plugin locator and a built-in source without one. Assert that serialization preserves the locator in the first case and omits the plugin field in the second.studio-ui/ui/app/src/components/ContentTypeManagement/components/PickControlDialog.tsx (1)
82-92: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regression coverage for root-only control filtering.
Add one test that keeps
metadata.rootOnlycontrols at the content-type root. Add another test that removes them for nested and repeat field paths. Cover both built-in controls andconfigDescriptors.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@studio-ui/ui/app/src/components/ContentTypeManagement/components/PickControlDialog.tsx` around lines 82 - 92, Add regression tests for the control list construction around typesFullList: verify metadata.rootOnly built-in controls and configDescriptors remain available at the content-type root, and verify both are excluded when fieldIdPath represents nested and repeat fields. Keep non-root-only controls included in each scenario.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/keyValueList.ts`:
- Around line 29-30: Validate the JSON.parse result in the key-value-list
parsing flow before calling map, rejecting falsy primitives such as null, false,
0, and "" while preserving an empty array as a valid empty-list result. Update
the relevant parser function and remove the now-unused asArray import.
---
Nitpick comments:
In
`@studio-ui/ui/app/src/components/ContentTypeManagement/components/PickControlDialog.tsx`:
- Around line 82-92: Add regression tests for the control list construction
around typesFullList: verify metadata.rootOnly built-in controls and
configDescriptors remain available at the content-type root, and verify both are
excluded when fieldIdPath represents nested and repeat fields. Keep
non-root-only controls included in each scenario.
In
`@studio-ui/ui/app/src/components/ContentTypeManagement/descriptors/controls/expiredDate.ts`:
- Around line 142-144: Add regression coverage for the expiredDate descriptor
bootstrap using field id expired_dt: verify normal and repeat/embedded
initialization create the expired_dt_tz atom, and verify the timezone value
remains intact through value parsing and subsequent form updates.
In `@studio-ui/ui/app/src/components/ContentTypeManagement/utils.ts`:
- Line 725: Add serialization coverage for the data-source handling around the
plugin field, testing both a plugin-backed source with a plugin locator and a
built-in source without one. Assert that serialization preserves the locator in
the first case and omits the plugin field in the second.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2ece4093-7bc2-4113-a4bb-d06e3dd778b8
📒 Files selected for processing (35)
studio-ui/.cursor/rules/type-builder-forms-engine.mdcstudio-ui/docs/type-builder-forms-engine-plugins.mdstudio-ui/docs/type-builder-forms-engine.mdstudio-ui/ui/app/src/components/ContentTypeManagement/components/EditTypeView.tsxstudio-ui/ui/app/src/components/ContentTypeManagement/components/PickControlDialog.tsxstudio-ui/ui/app/src/components/ContentTypeManagement/descriptors/controls/expiredDate.tsstudio-ui/ui/app/src/components/ContentTypeManagement/descriptors/controls/pageNavOrder.tsstudio-ui/ui/app/src/components/ContentTypeManagement/utils.tsstudio-ui/ui/app/src/components/FormsEngine/FormsEngine.tsxstudio-ui/ui/app/src/components/FormsEngine/components/GroupedDataSourceActionMenuItems.tsxstudio-ui/ui/app/src/components/FormsEngine/controls/ImagePicker.tsxstudio-ui/ui/app/src/components/FormsEngine/controls/PageNavOrder.tsxstudio-ui/ui/app/src/components/FormsEngine/controls/TranscodedVideoPicker.tsxstudio-ui/ui/app/src/components/FormsEngine/controls/VideoPicker.tsxstudio-ui/ui/app/src/components/FormsEngine/dataSourceHooks/useDataSourceListOptions.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/actionAdapters.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/bindings.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/moduleHelpers.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/audioBrowseRepo.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/audioDesktopUpload.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/imgDesktopUpload.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/imgRepositoryUpload.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/keyValueList.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/sharedContent.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/simpleTaxonomy.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/videoBrowseRepo.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/modules/videoDesktopUpload.tsstudio-ui/ui/app/src/components/FormsEngine/dataSources/types.tsstudio-ui/ui/app/src/components/FormsEngine/lib/controlPluginLoader.tsstudio-ui/ui/app/src/components/FormsEngine/lib/formUtils.tsxstudio-ui/ui/app/src/components/FormsEngine/lib/rteUtils.tsstudio-ui/ui/app/src/components/FormsEngine/lib/valueRetrievers.tsstudio-ui/ui/app/src/hooks/useVideoInfo.tsstudio-ui/ui/app/src/services/plugin.tsstudio-ui/ui/app/src/state/reducers/uiConfig.ts
🚧 Files skipped from review as they are similar to previous changes (28)
- studio-ui/ui/app/src/components/ContentTypeManagement/descriptors/controls/pageNavOrder.ts
- studio-ui/.cursor/rules/type-builder-forms-engine.mdc
- studio-ui/ui/app/src/components/FormsEngine/lib/rteUtils.ts
- studio-ui/ui/app/src/components/FormsEngine/FormsEngine.tsx
- studio-ui/ui/app/src/components/FormsEngine/lib/valueRetrievers.ts
- studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/videoDesktopUpload.ts
- studio-ui/ui/app/src/components/ContentTypeManagement/components/EditTypeView.tsx
- studio-ui/docs/type-builder-forms-engine.md
- studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/videoBrowseRepo.ts
- studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/imgDesktopUpload.ts
- studio-ui/ui/app/src/services/plugin.ts
- studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/audioDesktopUpload.ts
- studio-ui/ui/app/src/components/FormsEngine/lib/formUtils.tsx
- studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/sharedContent.ts
- studio-ui/docs/type-builder-forms-engine-plugins.md
- studio-ui/ui/app/src/components/FormsEngine/dataSourceHooks/useDataSourceListOptions.ts
- studio-ui/ui/app/src/state/reducers/uiConfig.ts
- studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/simpleTaxonomy.ts
- studio-ui/ui/app/src/components/FormsEngine/components/GroupedDataSourceActionMenuItems.tsx
- studio-ui/ui/app/src/components/FormsEngine/dataSources/actionAdapters.ts
- studio-ui/ui/app/src/components/FormsEngine/controls/PageNavOrder.tsx
- studio-ui/ui/app/src/components/FormsEngine/dataSources/bindings.ts
- studio-ui/ui/app/src/components/FormsEngine/controls/TranscodedVideoPicker.tsx
- studio-ui/ui/app/src/components/FormsEngine/dataSources/types.ts
- studio-ui/ui/app/src/components/FormsEngine/dataSources/modules/audioBrowseRepo.ts
- studio-ui/ui/app/src/components/FormsEngine/controls/ImagePicker.tsx
- studio-ui/ui/app/src/components/FormsEngine/lib/controlPluginLoader.ts
- studio-ui/ui/app/src/components/FormsEngine/controls/VideoPicker.tsx
…st path, enforce descriptor id comparison, and preflight plugin contributions before registry commit.
…stability, capabilities, and creation contracts.
…ler` lifecycle, async hook behaviour, type-local `form-controller.js` location, and authenticated loading mechanisms. Refine distinctions from legacy FE1 controllers and plugin-based controls.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@studio-ui/docs/type-builder-forms-engine.md`:
- Around line 629-633: Update the open convergence-gap checklist in the
documentation to add unchecked entries for S3/WebDAV capability stubs,
non-rendering disabled/internal-name/link-input/link-textarea controls, missing
FE2 Crafter-specific RTE plugin equivalents, focused compatibility tests,
importPlugin() descriptor ownership validation, and non-atomic plugin
registration. Add a corresponding Progress entry summarizing these findings,
while keeping completed design decisions separate from remaining implementation
and validation work.
- Around line 483-495: Add the text language tag to both fenced code blocks in
the form-controller documentation: the authoring artifact path example and the
authenticated GET request example. Leave their contents unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fecffe57-5a53-44e9-bbe1-4f3a4eccffd1
📒 Files selected for processing (1)
studio-ui/docs/type-builder-forms-engine.md
…sign details, async hook definitions, loading process, and usage context. Document completed decisions, remaining implementation, and integration points.
rart
changed the title
… code examples in `form-controller.js`.
Contributor
Author
|
@coderabbitai I'm done with this for now. Ok to hand this over for human review? |
|
Tip For best results, initiate chat on the files or code changes.
The PR provides a sound architectural foundation for FE2 data sources and control plugins. The human review should specifically assess the known boundaries:
The development-draft notice in ✏️ Learnings added
You are interacting with an AI system. |
sumerjabri
approved these changes
Aug 4, 2026
This was referenced Aug 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces FE2’s hard-coded data-source type switches with a pluggable module + actions/capabilities runtime, and consolidates FE control/DS plugins onto the existing Studio
PluginDescriptormodel (importPlugin→registerPlugin).Data-source runtime
FormsEngine/dataSources/: versionedDataSourceModule, instance factory, capabilities (browse/search/upload/create/list/ …), declarative actions, andControlProps.dataSourcesinjection viauseFieldDataSources.DataSourceServices(browseFiles/search/upload/ create/push form).ds.type:instance.listviauseDataSourceListOptionsdataSourceMapas the primary path; kept adapters only where NodeSelector still needs a presentation summary.dataSources.errorsfor unknown ids/types/plugin failures.Single plugin model
PluginDescriptor; the host registers contributions (no self-register side effects).PluginDescriptor.dataSources→ installed into the DS module registry onregisterPlugin.PluginDescriptor.controls→ installed into the control contribution registry;controlPluginLoaderdemand-loads viaimportPluginand looks up byfield.type(same pattern as DSrecord.type).<plugin>locators unchanged; TB still copies coords onto fields/DS records.Correctness fixes along the way
get_content_by_commit_idloops by stabilizinguseFieldDataSourcesdeps (content-types keyed by value, not identity churn).shared-contentwith empty Default Type (allowedCreatePaths+ create-picker path attribution / null-safe loading).contentTypes: '*'can emit create targets.Docs & samples
docs/type-builder-forms-engine.md,docs/type-builder-forms-engine-plugins.md, control handoffdocs/fe2-control-plugin-single-model-implementation.md.Still open (not in this PR)
PluginDescriptor.utils.datasourceprovenance into FE2/XB XML.Test plan
get_content_by_commit_idloops on form opendataSources/controlsloads via form-definition plugin coords (if a test plugin is installed)yarn compileinui/appNotes
This PR includes:
Which correspond to:
Summary by CodeRabbit