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

Fix vscode api enum #333

Merged
merged 2 commits into from
Mar 21, 2022
Merged

Fix vscode api enum #333

merged 2 commits into from
Mar 21, 2022

Conversation

CGNonofr
Copy link
Collaborator

fix for #332

@CGNonofr
Copy link
Collaborator Author

@kaisalmen I think this fix is pretty urgent

Copy link
Collaborator

@kaisalmen kaisalmen left a comment

Choose a reason for hiding this comment

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

👍 import type is a very good idea. Just have a look at my question. Thanks.

@@ -119,6 +119,14 @@ export function createVSCodeApi(servicesProvider: Services.Provider): typeof vsc
}
}

enum TextDocumentChangeReason {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Question for my understanding: We have to redefine the enum because otherwise vscode api is exposed in the listener?

Copy link
Collaborator Author

@CGNonofr CGNonofr Mar 21, 2022

Choose a reason for hiding this comment

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

The error was at line reason: isUndoing ? vscode.TextDocumentChangeReason.Undo : isRedoing ? vscode.TextDocumentChangeReason.Redo : undefined

vscode.TextDocumentChangeReason wasn't defined so it was crashing

I hope I understood your question

Copy link
Collaborator

@kaisalmen kaisalmen Mar 21, 2022

Choose a reason for hiding this comment

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

Yes, you understood me. Thanks. 👍🙂 Just by looking at the PR without having deep knowledge of the project yet doesn't make the problem directly clear. That's why import type (did not know it and looked it up) is very helpful here as it immediately makes the issue obvious.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍 The same way we need to re-implement all vscode.* interfaces, enums in typescript are transpiled into objects, and we need those at runtime

@kaisalmen
Copy link
Collaborator

I will merge and release 0.18.1.

@kaisalmen kaisalmen merged commit 5ff78ea into master Mar 21, 2022
@CGNonofr
Copy link
Collaborator Author

CGNonofr commented Mar 21, 2022

I will merge and release 0.18.1.

you rock!

@kaisalmen
Copy link
Collaborator

Done ✅

@CGNonofr
Copy link
Collaborator Author

Done white_check_mark

Great! thank you and my apologies for the bug!

btw, any news for the other MR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants