Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uplift Monaco to VSCode v. 1.65.2 #10736

Merged
merged 78 commits into from
Mar 23, 2022

Conversation

colin-grant-work
Copy link
Contributor

@colin-grant-work colin-grant-work commented Feb 10, 2022

What it does

Fixes #10387.

This PR has two main objectives:

  • Update our code to use Monaco@32.1(+)

The (+) indicates that the actual code at @theia/monaco-editor-core@0.32.1-alpha0 is ahead of the Monaco release 0.32.1. The current plan is to wait for the next VSCode release, then peg our code to that, and perhaps label @theia/monaco-editor-core with the VSCode version number rather than the monaco-editor-core version number.

  • Update our code to use Webpackable modules, rather than Asynch Module code with custom loaders. This allows us to be transparent about what Monaco code is being consumed where in our code base, and makes it easier to know where we're using public API (import * as Monaco from '@theia/monaco-editor-core') vs. private API (import { whatever } from '@theia/monaco-editor-core/*')

In addition, it includes the TypeScript source in the @theia/monaco-editor-core NPM package, so it is possible to simply click through to definitions in the source, making it easy to explore the Monaco code running in Theia.

This PR does not (for the most part):

  • change the way we're using Monaco, clean up (much) Monaco-related code, or implement new features with the Monaco functionality. There is plenty of scope for this work, but it seems best to separate the uplift itself from new feature development.

I've added a file called uplift-journal.md in packages/monaco - have a look there for some observations about the process. I will be migrating that content to documentation in the wiki over the next little while.

How to test

Testing instructions can be found here.

Review checklist

Reminder for reviewers

@colin-grant-work
Copy link
Contributor Author

@msujew, this is still a ways off from being ready, but one problem that's arisen is that even strings copied from Monaco at a newer version aren't present in our current nls.metadata.json. Do you have any thoughts about how we want to maintain that file and handle changes in our Monaco and VSCode API targets / dependencies?

@colin-grant-work colin-grant-work changed the title Uplift Monaco to ca. 0.32.1 Uplift Monaco to VSCode v. 1.65.2 Mar 16, 2022
@colin-grant-work
Copy link
Contributor Author

@planger, the one CI check failing here is Playwright, and as we've discussed, it's the change to the default value of editor.autoSave in the browser that's throwing the test off by saving the editor when it's expected to be dirty. How would you like to address that change?

@planger
Copy link
Contributor

planger commented Mar 16, 2022

@colin-grant-work I would suggest to extend the TheiaEditor in order to allow setting the editor.autoSave preference via the main menu and then toggle it off before the test in beforeAll.

But, please feel free to just skip the test in this PR, so your PR is green, and I'll open an issue that we'll resolve soon. WDYT?

@colin-grant-work
Copy link
Contributor Author

@planger, thanks for the input. I have disabled the test for now, and it looks like @ndoschek has already created the corresponding issue so thanks to her, as well.

This was referenced Mar 18, 2022
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add my approvement as well, since I already gave it unofficially at the dev-meeting. The change is looking good to me, thanks Colin!

Also
 - removes monaco/src/typings/monaco/index.d.ts which was restored in a merge
 - cleans up formating of `plugin-metrics-languages-main.ts
 - removes now-unused copy plugin import from webpack-generator.ts
@colin-grant-work colin-grant-work added this to the 1.24.0 milestone Mar 23, 2022
@colin-grant-work colin-grant-work merged commit a19ce98 into eclipse-theia:master Mar 23, 2022
planger added a commit to planger/theia that referenced this pull request Mar 25, 2022
Due to the Monaco uplift, we had to temporarily skip a test, because
it affected the default preference regarding auto-save.

To re-enable the test, we now explicitly set the auto save preference to
`off` before the test. This way the preference value is ensured to be
consistent.

See also eclipse-theia#10736

Fixes eclipse-theia#10891

Change-Id: I6ab22a71848e174313a30f9121cb4b3dec363f12
planger added a commit to planger/theia that referenced this pull request Mar 25, 2022
Due to the Monaco uplift, we had to temporarily skip a test, because
it affected the default preference regarding auto-save.

To re-enable the test, we now explicitly set the auto save preference to
`off` before the test. This way the preference value is ensured to be
consistent.

See also eclipse-theia#10736

Fixes eclipse-theia#10891

Change-Id: I6ab22a71848e174313a30f9121cb4b3dec363f12
planger added a commit to planger/theia that referenced this pull request Mar 30, 2022
Due to the Monaco uplift, we had to temporarily skip a test, because
it affected the default preference regarding auto-save.

To re-enable the test, we now explicitly set the auto save preference to
`off` before the test. This way the preference value is ensured to be
consistent.

See also eclipse-theia#10736

Fixes eclipse-theia#10891

Change-Id: I6ab22a71848e174313a30f9121cb4b3dec363f12
planger added a commit to eclipsesource/theia that referenced this pull request Apr 29, 2022
Due to the Monaco uplift, we had to temporarily skip a test, because
it affected the default preference regarding auto-save.

To re-enable the test, we now explicitly set the auto save preference to
`off` before the test. This way the preference value is ensured to be
consistent.

See also eclipse-theia#10736

Fixes eclipse-theia#10891

Change-Id: I6ab22a71848e174313a30f9121cb4b3dec363f12
planger added a commit to planger/theia that referenced this pull request May 5, 2022
Due to the Monaco uplift, we had to temporarily skip a test, because
it affected the default preference regarding auto-save.

To re-enable the test, we now explicitly set the auto save preference to
`off` before the test. This way the preference value is ensured to be
consistent.

In order to do so, we add support for option preferences

See also eclipse-theia#10736

Fixes eclipse-theia#10891

Change-Id: I6ab22a71848e174313a30f9121cb4b3dec363f12
planger added a commit to planger/theia that referenced this pull request May 10, 2022
Due to the Monaco uplift, we had to temporarily skip a test, because
it affected the default preference regarding auto-save.

To re-enable the test, we now explicitly set the auto save preference to
`off` before the test. This way the preference value is ensured to be
consistent.

In order to do so, we add support for option preferences

See also eclipse-theia#10736

Fixes eclipse-theia#10891

Change-Id: I6ab22a71848e174313a30f9121cb4b3dec363f12
JonasHelming pushed a commit that referenced this pull request May 10, 2022
Due to the Monaco uplift, we had to temporarily skip a test, because
it affected the default preference regarding auto-save.

To re-enable the test, we now explicitly set the auto save preference to
`off` before the test. This way the preference value is ensured to be
consistent.

In order to do so, we add support for option preferences

See also #10736

Fixes #10891

Change-Id: I6ab22a71848e174313a30f9121cb4b3dec363f12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
monaco issues related to monaco
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Monaco
7 participants