Skip to content

fix: bundling error makes terminal suggestions fail#208822

Merged
Tyriar merged 2 commits intomicrosoft:mainfrom
cpendery:fix/bundle-suggestion-error
Mar 28, 2024
Merged

fix: bundling error makes terminal suggestions fail#208822
Tyriar merged 2 commits intomicrosoft:mainfrom
cpendery:fix/bundle-suggestion-error

Conversation

@cpendery
Copy link
Member

@cpendery cpendery commented Mar 26, 2024

There is an issue with the bundling / build process for VSCode Insiders that introduces an undefined reference bug to the SimpleSuggestWidget show below.

bundleError.mp4

As you can see in the code snippets below and their bundled counterparts, the second bundled image of calling showSuggestions uses the variable this.h._completionModel, but it should be this.h.b as seen in the first bundled image. This results in the value being undefined and causing the suggestions to error out. This PR removes the passing of a private variable to the widget and instead exposes a set method for the CompletionModel which should fix this issue in the build.

I'm not sure how to test this as it was working fine in the development environment, but removing the private access seems like it would fix the issue.

 

Update the widget's model

Code

showSuggestions(completionModel: SimpleCompletionModel, selectionIndex: number, isFrozen: boolean, isAuto: boolean, cursorPosition: { top: number; left: number; height: number }): void {
this._cursorPosition = cursorPosition;
// this._contentWidget.setPosition(this.editor.getPosition());
// this._loadingTimeout?.dispose();
// this._currentSuggestionDetails?.cancel();
// this._currentSuggestionDetails = undefined;
if (this._completionModel !== completionModel) {
this._completionModel = completionModel;
}

Bundled Code

bundleBundled

Call showSuggestions

Code

this._suggestWidget?.showSuggestions((this._suggestWidget as any)._completionModel, 0, false, false, {
left: (xtermBox.left - panelBox.left) + (this._terminal.buffer.active.cursorX + handledCursorDelta) * dimensions.width,
top: (xtermBox.top - panelBox.top) + this._terminal.buffer.active.cursorY * dimensions.height,
height: dimensions.height
});

Bundled Code

bundleCallsite

Part of #154662

@Tyriar
Copy link
Member

Tyriar commented Mar 26, 2024

Nice investigation 🙂 we're in the test phase so I'll try get this merged later this week or early next week

@Tyriar Tyriar added this to the April 2024 milestone Mar 26, 2024
// TODO: What do frozen and auto do?
const xtermBox = this._screen!.getBoundingClientRect();
const panelBox = this._panel!.offsetParent!.getBoundingClientRect();
this._suggestWidget?.showSuggestions((this._suggestWidget as any)._completionModel, 0, false, false, {
Copy link
Member

Choose a reason for hiding this comment

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

Pretty sure this any was the cause of the problem, we do our own private member mangling so access to private methods like this does not work.

@Tyriar Tyriar enabled auto-merge March 28, 2024 16:33
@Tyriar Tyriar merged commit 78447f9 into microsoft:main Mar 28, 2024
@microsoft microsoft locked and limited conversation to collaborators Jun 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants