chore: Add code-split for OPL with UI module instances #40749
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant UpdateLayoutServiceCEImpl
Caller->>UpdateLayoutServiceCEImpl: updateLayoutDsl(creatorId, creatorType, dsl)
UpdateLayoutServiceCEImpl->>UpdateLayoutServiceCEImpl: processDslAndCreateSyntheticWidgets(...)
UpdateLayoutServiceCEImpl-->>UpdateLayoutServiceCEImpl: (processedDsl, syntheticWidgetNames)
UpdateLayoutServiceCEImpl->>UpdateLayoutServiceCEImpl: stripSyntheticWidgets(processedDsl, syntheticWidgetNames)
UpdateLayoutServiceCEImpl->>UpdateLayoutServiceCEImpl: Extract widget names, handle dynamic bindings
UpdateLayoutServiceCEImpl->>UpdateLayoutServiceCEImpl: Update on-load executables, run behavior
UpdateLayoutServiceCEImpl-->>Caller: Final processed DSL
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
subrata71
added
ok-to-test
github-actions
bot
added
the
skip-changelog
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/15248942088. |
subrata71
added
ok-to-test
|
Deploy-Preview-URL: https://ce-40749.dp.appsmith.com |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/server/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceCEImpl.java (2)
114-118: Document the extension point clearly.This method serves as an extension point for UI module handling but currently returns no synthetic widgets. Consider adding comprehensive JavaDoc to clarify its purpose and expected behavior in subclasses.
+ /** + * Extension point for UI module implementations to process DSL and inject synthetic widgets. + * + * @param creatorId The creator ID + * @param creatorType The creator context type + * @param dsl The DSL to process + * @param mainChildren The main children array from the DSL + * @return A Mono containing a Map.Entry with the processed DSL and optional synthetic widget names + */ protected Mono<Map.Entry<JSONObject, Optional<Set<String>>>> processDslAndCreateSyntheticWidgets( String creatorId, CreatorContextType creatorType, JSONObject dsl, ArrayList<Object> mainChildren) { return Mono.just(Map.entry(dsl, Optional.empty())); }
286-288: Add documentation for the placeholder method.This method currently serves as a placeholder for synthetic widget removal logic. Consider documenting its intended behavior for future implementations.
+ /** + * Placeholder method for removing synthetic widgets from the DSL. + * Override in subclasses to implement specific synthetic widget removal logic. + * + * @param dsl The DSL to process + * @param syntheticNamesOpt Optional set of synthetic widget names to remove + * @return The processed DSL with synthetic widgets removed + */ protected JSONObject stripSyntheticWidgets(JSONObject dsl, Optional<Set<String>> syntheticNamesOpt) { return dsl; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceCEImpl.java(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: perform-test / ci-test-result
- GitHub Check: server-spotless / spotless-check
- GitHub Check: server-unit-tests / server-unit-tests
🔇 Additional comments (4)
app/server/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceCEImpl.java (4)
27-27: LGTM! New imports are appropriate for the functionality.The added imports for
Setsutility andOptionaltype are correctly used in the synthetic widget processing logic.Also applies to: 47-47
132-141: Well-designed setup for synthetic widget processing.The logic correctly preserves the original DSL structure when no UI module instances are present, which maintains backward compatibility for existing tests.
168-182: Synthetic widget filtering logic is correctly implemented.The use of
Sets.differenceto filter out synthetic widgets from the main widget names is appropriate, and the conditional logic properly handles both cases (with and without synthetic widgets).
138-283: Excellent refactoring that maintains existing functionality while adding extension points.The refactored logic correctly:
- Processes DSL through the new extension point
- Handles synthetic widget filtering when present
- Preserves all existing error handling and reactive patterns
- Maintains the same observable spans and analytics events
The architecture provides clean extension points for UI module implementations while ensuring backward compatibility.
3 participants
Description
EE Counterpart PR: https://github.com/appsmithorg/appsmith-ee/pull/7559
Fixes #
Issue Numberor
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/15253633122
Commit: dd70a1c
Cypress dashboard.
Tags:
@tag.AllSpec:
Mon, 26 May 2025 13:55:15 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit