Development: Update monaco-editor to 0.52.0
#9431
Conversation
github-actions
bot
added
the
client
pzdr7
temporarily deployed
to
artemis-test6.artemis.cit.tum.de
GitHub Actions
Inactive
pzdr7
temporarily deployed
to
artemis-test6.artemis.cit.tum.de
GitHub Actions
Inactive
src/main/webapp/app/shared/monaco-editor/model/actions/adapter/monaco-text-editor.adapter.ts
Show resolved
Hide resolved
WalkthroughThe changes in this pull request involve updates to the configuration and functionality of the Monaco Editor within an Angular project. Key modifications include changing asset paths in Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
- angular.json (1 hunks)
- package.json (1 hunks)
- prebuild.mjs (2 hunks)
- src/main/webapp/app/core/config/monaco.config.ts (1 hunks)
- src/main/webapp/app/shared/monaco-editor/model/actions/adapter/monaco-text-editor.adapter.ts (1 hunks)
- src/main/webapp/app/shared/monaco-editor/model/actions/adapter/text-editor.interface.ts (1 hunks)
- src/main/webapp/app/shared/monaco-editor/model/actions/communication/channel-reference.action.ts (1 hunks)
- src/main/webapp/app/shared/monaco-editor/model/actions/communication/exercise-reference.action.ts (1 hunks)
- src/main/webapp/app/shared/monaco-editor/model/actions/communication/user-mention.action.ts (1 hunks)
- src/main/webapp/app/shared/monaco-editor/model/actions/text-editor-action.model.ts (0 hunks)
💤 Files with no reviewable changes (1)
- src/main/webapp/app/shared/monaco-editor/model/actions/text-editor-action.model.ts
🧰 Additional context used
📓 Path-based instructions (6)
src/main/webapp/app/core/config/monaco.config.ts (1)
src/main/webapp/app/shared/monaco-editor/model/actions/adapter/monaco-text-editor.adapter.ts (1)
src/main/webapp/app/shared/monaco-editor/model/actions/adapter/text-editor.interface.ts (1)
src/main/webapp/app/shared/monaco-editor/model/actions/communication/channel-reference.action.ts (1)
src/main/webapp/app/shared/monaco-editor/model/actions/communication/exercise-reference.action.ts (1)
src/main/webapp/app/shared/monaco-editor/model/actions/communication/user-mention.action.ts (1)
📓 Learnings (1)
src/main/webapp/app/shared/monaco-editor/model/actions/communication/user-mention.action.ts (1)
Learnt from: pzdr7 PR: ls1intum/Artemis#9124 File: src/main/webapp/app/shared/monaco-editor/model/actions/communication/monaco-user-mention.action.ts:63-65 Timestamp: 2024-07-28T20:43:40.544Z Learning: In the `MonacoUserMentionAction` class, the context ensures that the course ID is always available, making the use of non-null assertion acceptable in this specific scenario.
🪛 Biome
prebuild.mjs
[error] 8-8: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
[error] 9-9: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
[error] 11-11: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
🔇 Additional comments (7)
src/main/webapp/app/shared/monaco-editor/model/actions/communication/user-mention.action.ts (1)
48-49: LGTM: Improved method documentationThe updated documentation for the
runmethod provides a more accurate description of the action performed. It now correctly states that the method "inserts" the text rather than "types" it, which aligns better with the actual implementation. This change improves clarity and maintains consistency between the documentation and the code.src/main/webapp/app/shared/monaco-editor/model/actions/communication/exercise-reference.action.ts (1)
55-56: LGTM: Documentation update is clear and accurate.The change in wording from "Types" to "Inserts" more accurately reflects the action being performed by the
runmethod. The documentation is concise and follows the project's coding guidelines.src/main/webapp/app/shared/monaco-editor/model/actions/communication/channel-reference.action.ts (2)
47-49: LGTM: Documentation update is accurate and clear.The updated comment accurately reflects the new behavior of the
runmethod. This change improves the clarity of the code and aligns with best practices for maintaining up-to-date documentation.
50-52: LGTM: Improved editor interaction and user experience.The changes in the
runmethod enhance the functionality:
- Using
replaceTextAtCurrentSelectioninstead oftypeTextlikely provides better control over text insertion.- Adding
editor.triggerCompletion()ensures immediate triggering of the completion provider, improving the user experience.These modifications align with the coding guidelines and improve the overall functionality of the action.
src/main/webapp/app/shared/monaco-editor/model/actions/adapter/text-editor.interface.ts (1)
45-47: 🧹 Nitpick (assertive)LGTM! Consider adding a comment for clarity.
The addition of
triggerCompletion()method aligns with the PR objectives of addressing issues with code completions. This change improves the interface by providing a dedicated method for triggering completions.Consider adding a brief comment to explain the purpose and expected behavior of this method. For example:
/** * Triggers the completion in the editor, e.g. by showing a widget. * This method is used to programmatically invoke the editor's completion functionality. */ triggerCompletion(): void;The removal of the
typeTextmethod might impact other parts of the application. Let's verify its usage:✅ Verification successful
Verified: No remaining usages of the
typeTextmethod found.The removal of
typeTextdoes not impact any parts of the application. The addition oftriggerCompletion()aligns with the intended improvements.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining usage of the removed typeText method # Test: Search for typeText method calls echo "Searching for typeText method calls:" rg --type typescript 'typeText\(' src/main/webappLength of output: 169
Script:
#!/bin/bash # Description: Check for any remaining usage of the removed typeText method # Test: Search for typeText method calls in .ts and .tsx files echo "Searching for typeText method calls:" rg 'typeText\(' --glob '*.ts' --glob '*.tsx' src/main/webappLength of output: 145
package.json (1)
61-61: LGTM: Monaco Editor updated to version 0.52.0The update of
monaco-editorfrom version 0.51.0 to 0.52.0 aligns with the PR objectives. This change should address the mentioned web worker errors and the minor bug with code completions.To ensure a smooth transition, please verify:
- Compatibility with other dependencies.
- Any necessary configuration updates in other files.
You can use the following script to check for other files that might need updates due to this change:
✅ Verification successful
Verified: Monaco Editor successfully updated to version 0.52.0
The
monaco-editordependency was updated, and no files contain version-specific references to the previous version. The update aligns with the PR objectives and should resolve the reported issues.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for files that might need updates due to Monaco Editor version change # Test: Search for files referencing Monaco Editor echo "Files potentially affected by Monaco Editor update:" rg --type-not json "monaco-editor" -l # Test: Check for version-specific imports or configurations echo "\nFiles with version-specific Monaco Editor references:" rg --type-not json "monaco-editor.*0\.51\.0" -lLength of output: 12685
src/main/webapp/app/shared/monaco-editor/model/actions/adapter/monaco-text-editor.adapter.ts (1)
155-156: ThetriggerCompletionmethod correctly triggers the suggestion widgetThe implementation appropriately invokes the suggestion widget using
editor.trigger, which addresses the issue where typing no longer suffices to trigger completions.
src/main/webapp/app/shared/monaco-editor/model/actions/communication/user-mention.action.ts
Show resolved
Hide resolved
...ain/webapp/app/shared/monaco-editor/model/actions/communication/exercise-reference.action.ts
Show resolved
Hide resolved
magaupp
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
GitHub Actions
Inactive
magaupp
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
GitHub Actions
Inactive
There was a problem hiding this comment.
Tested on TS4.
"Review changes" diff view loads properly. Test cases are completed in the problem statement editor. Clicking the @, #, and "Exercise" buttons opens the completion popup.
Unrelated to this PR:
I noticed that the @ name completion does not show the correct names in a PostgreSQL deployment.
Also, clicking on "Disable split view" in the "Review Changes" modal does not work at the first try.
az108
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
GitHub Actions
Inactive
Checklist
General
Client
Changes affecting Programming Exercises
Motivation and Context
After upgrading to
0.52.0, the Monaco editor would sometimes encounter errors related to its web worker, which would cause it to lose its language features.Potentially related to microsoft/monaco-editor#4694, although it's not 100% clear if this is the same error
Description
monaco-editoresbuild sample (https://github.com/microsoft/monaco-editor/blob/main/samples/browser-esm-esbuild/build.js)monaco-editorto version 0.52.0typeactionSteps for Testing
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Code Review
Manual Tests
Test Coverage
Unchanged
Screenshots
No UI changes
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
monaco-editordependency to the latest version for improved performance and features.