Skip to content

Source Editor refactoring integration

Denys Mishunov requested to merge 292943-se-integration into master

What does this MR do and why?

This is the integration MR for the Source Editor refactoring. It is the last leg of a big effort for refactoring the architecture of Source Editor and the extensions introduced in the dedicated PoC:

  1. Introduce a separate module for Source Editor extensions (merged)
  2. Introduce a separate module for Source Editor instance
  3. Switch existing Source Editor architecture to use the new modules [this MR]
    • update source_editor.js to replace the current instance and extension implementations with the new one
    • update all existing extensions to follow the introduced changes

Can't we split this MR into smaller ones?

Unfortunately, no, we can not. This MR contains refactoring to Source Editor core. Due to these changes, we have to update all of the existing extensions to match the new architecture. Since all the extensions are updated, we also have to update all the use-cases where these extensions are used. And of course, all of the tests should be updated in the same go.

TL;DR; of the new architecture

General info

  • The new architecture of Source Editor forces a clear and straightforward API for extensions. An example of the new extension's structure and its API can be observed in ~/editor/example_source_editor_extension.js
  • The Source Editor Instance and Extension are now the separate modules
  • The Source Editor Instance 🎉 is not mutated anymore 🎉 Previously, we were following the mixin pattern where we really mutated instance originally created by Monaco. Even without proper name-conflict resolutions. Now, the Source Instance is a proxy connecting underlying Monaco instance with very thin own layer (use and unuse) and all the functionality, introduced by the extensions
  • Cleanup of the Source Editor functionality that has not been used
  • All used extensions are stored in the global extensions registry on the global editor (see #292943 (closed))

Extensions

  • It is possible to unuse extensions now (see #341021 (closed)). It's also possible to do it dynamically based on the environment!
  • Introduced a very basic name-conflict resolution (simple throw for now) in case an extension tries to introduce the same-named method as Monaco or another extension. The conflicting extension doesn't get used in this case.
  • Extensions are made as independent as possible. Hence the base extension should be explicitly used now instead of being extended

Extension's public API

Testing extensions

  • Since the instance now is a proxy it's not easy to spy on methods, provided by that or another extension. To address that, the new spyOnApi helper has been introduced. The examples of usage can be found in the specs in this MR

Screenshots or screen recordings

No visual changes

How to set up and validate locally

Technically, everything should work exactly as it used to be 🤞

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #292943 (closed)

Edited by Denys Mishunov

Merge request reports