CodeEditor: Ensure suggestions only apply to the instance of the editor that registered them #69995
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lastFile:packages/grafana-ui/src/components/Monaco/CodeEditor.internal.story.tsx
lastFile:packages/grafana-ui/src/components/Monaco/suggestions.ts
lastFile:packages/grafana-ui/src/components/Monaco/CodeEditor.internal.story.tsx
lastFile:packages/grafana-ui/src/components/Monaco/suggestions.ts
ashharrison90
requested review from
tskarhed and
L-M-K-B
and removed request for
a team
ashharrison90
added
area/frontend/code-editor
mob session
tskarhed
reviewed
Jun 13, 2023
Comment on lines
-90
to
-92
| if (getSuggestions) { | ||
| this.completionCancel = registerSuggestions(monaco, language, getSuggestions); | ||
| } |
There was a problem hiding this comment.
What is the logical difference ebtween doing this onMount instead of beforeMount?
There was a problem hiding this comment.
i guess it will happen slightly later, but by doing it onMount we get access to the editor instance, whereas before mount we only have access to the global monaco instance
|
|
||
| this.modelId = editor.getModel()?.id; |
There was a problem hiding this comment.
This doesn't require a URI? 😮
There was a problem hiding this comment.
nope, i think because the model is associated with this editor (as opposed to getting it from the global monaco namespace)
tskarhed
approved these changes
Jun 13, 2023
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-69995-to-v10.0.x origin/v10.0.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x 61dbad6905264e41e9464032f065993fb50cf4ea
# When the conflicts are resolved, stage and commit the changes
git add . && git commit --no-edit
# Push it to GitHub
git push --set-upstream origin backport-69995-to-v10.0.x
git switch main
# Remove the local backport branch
git branch -D backport-69995-to-v10.0.xThen, create a pull request where the |
grafanabot
added
the
backport-failed
ashharrison90
added a commit
that referenced
this pull request
Jun 14, 2023
…or that registered them (#69995) * user essentials mob! 🔱 lastFile:packages/grafana-ui/src/components/Monaco/CodeEditor.internal.story.tsx * user essentials mob! 🔱 lastFile:packages/grafana-ui/src/components/Monaco/suggestions.ts * user essentials mob! 🔱 lastFile:packages/grafana-ui/src/components/Monaco/CodeEditor.internal.story.tsx * user essentials mob! 🔱 lastFile:packages/grafana-ui/src/components/Monaco/suggestions.ts * remove duplicate editor from story * remove suggestions from story --------- Co-authored-by: Laura Benz <laura.benz@grafana.com> Co-authored-by: Tobias Skarhed <tobias.skarhed@gmail.com> (cherry picked from commit 61dbad6) # Conflicts: # packages/grafana-ui/src/components/Monaco/CodeEditor.tsx
ashharrison90
added a commit
that referenced
this pull request
Jun 14, 2023
#70067) CodeEditor: Ensure suggestions only apply to the instance of the editor that registered them (#69995) * user essentials mob! 🔱 lastFile:packages/grafana-ui/src/components/Monaco/CodeEditor.internal.story.tsx * user essentials mob! 🔱 lastFile:packages/grafana-ui/src/components/Monaco/suggestions.ts * user essentials mob! 🔱 lastFile:packages/grafana-ui/src/components/Monaco/CodeEditor.internal.story.tsx * user essentials mob! 🔱 lastFile:packages/grafana-ui/src/components/Monaco/suggestions.ts * remove duplicate editor from story * remove suggestions from story --------- Co-authored-by: Laura Benz <laura.benz@grafana.com> Co-authored-by: Tobias Skarhed <tobias.skarhed@gmail.com> (cherry picked from commit 61dbad6) # Conflicts: # packages/grafana-ui/src/components/Monaco/CodeEditor.tsx
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is this feature?
modelIdwhen registering suggestionsprovideCompletionItemscan check themodelIdagainst the registeredmodelIdWhy do we need this feature?
Who is this feature for?
CodeEditorWhich issue(s) does this PR fix?:
Fixes #69262
Special notes for your reviewer:
i tried to look for a way to write a test that would prevent regressions to this... but i don't think it's possible in react-testing-library 😬
Please check that: