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

Downgrading monaco-editor to v0.31.1 #785

Merged
merged 3 commits into from
Sep 30, 2022

Conversation

mofojed
Copy link
Member

@mofojed mofojed commented Sep 28, 2022

I went down a rabbit hole debugging this...
The issue is that container is null when initializing the suggest widget contextview: https://github.com/microsoft/vscode/blob/0c22a33a9d670a84309447b36abdbd8c04ee6219/src/vs/base/browser/ui/contextview/contextview.ts#L162
Because it is null, this listener never gets registered: https://github.com/microsoft/vscode/blob/main/src/vs/base/browser/ui/contextview/contextview.ts#L183
Subsequently, this.hide() never gets called: https://github.com/microsoft/vscode/blob/0c22a33a9d670a84309447b36abdbd8c04ee6219/src/vs/base/browser/ui/contextview/contextview.ts#L352
I don't know how it's supposed to work since the container is always null for that service: https://github.com/microsoft/vscode/blob/0c22a33a9d670a84309447b36abdbd8c04ee6219/src/vs/editor/standalone/browser/standaloneLayoutService.ts#L27
I don't understand why this works in the Playground. I'm missing something, but for now I'm just going to roll back to v0.31.1 which doesn't have this issue.

- Fixes being unable to escape the suggestion widget
- I don't know what change introduced the issue, as it's not reproducible in v0.34.0
@mofojed mofojed added the bug Something isn't working label Sep 28, 2022
@mofojed mofojed added this to the September 2022 milestone Sep 28, 2022
@mofojed mofojed requested a review from mattrunyon September 28, 2022 22:39
@mofojed mofojed self-assigned this Sep 28, 2022
Not needed for this version of monaco
Comment on lines 24 to 29
window.MonacoEnvironment = {
getWorker(workerId, label) {
return editorWorker();
},
};

Copy link
Collaborator

@mattrunyon mattrunyon Sep 30, 2022

Choose a reason for hiding this comment

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

I'm guessing you removed this b/c of TS. You can use this instead to keep worker support. Monaco bumped their types at some point after 0.31 I think to extend window

import type { Environment } from 'monaco-editor';

declare global {
  interface Window {
    MonacoEnvironment: Environment;
  }
}

window.MonacoEnvironment = {
  getWorker() {
    return editorWorker();
  },
};

@codecov
Copy link

codecov bot commented Sep 30, 2022

Codecov Report

Merging #785 (1934d08) into main (ee5b246) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #785      +/-   ##
==========================================
- Coverage   36.60%   36.59%   -0.01%     
==========================================
  Files         412      412              
  Lines       30874    30879       +5     
  Branches     7598     7602       +4     
==========================================
- Hits        11302    11301       -1     
- Misses      19517    19523       +6     
  Partials       55       55              
Flag Coverage Δ
unit 36.59% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/console/src/monaco/MonacoUtils.ts 49.54% <0.00%> (ø)
packages/console/src/ConsoleInput.tsx 37.94% <0.00%> (-0.69%) ⬇️
packages/iris-grid/src/IrisGridProxyModel.ts 62.34% <0.00%> (-0.52%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@mattrunyon mattrunyon self-requested a review September 30, 2022 23:07
@mofojed mofojed merged commit b6853ac into deephaven:main Sep 30, 2022
@mofojed mofojed deleted the 773-ignore-suggestions branch September 30, 2022 23:23
@github-actions github-actions bot locked and limited conversation to collaborators Sep 30, 2022
@mofojed
Copy link
Member Author

mofojed commented Apr 6, 2023

Monaco issue is: microsoft/monaco-editor#2947

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Autocomplete suggestions cannot be ignored
2 participants