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

Create POST-MORTEM document #249

Merged
merged 1 commit into from
Apr 26, 2022
Merged

Create POST-MORTEM document #249

merged 1 commit into from
Apr 26, 2022

Conversation

MaxStalker
Copy link
Contributor

Closes: #239

Description

New document is created to capture all the valuable information, debugging processes and insights developers could get
during the process of Playground improvement.

Initial edit include information about recent work on #238.

@MaxStalker MaxStalker requested a review from 10thfloor as a code owner April 15, 2022 16:30
@vercel
Copy link

vercel bot commented Apr 15, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/onflow/flow-playground/869EKjN2oYiduRtXqdS98Knn8ogg
✅ Preview: https://flow-playground-git-max-lsp-post-mortem-onflow.vercel.app


We've used the same trick as previously implemented in VS Code extension - add new line character at the eof, them add
timeout and revert code to original state. This will "trick" editor into thinking that code was changed and need to be
checked for errors again, catching new changes on contract in the process. Win-win! :)

Choose a reason for hiding this comment

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

@MaxStalker if my understanding is correct -- the problem was when a user switched back to script while language server is not fully done with processing the changes made, it would result in error.
But is the solution mentioned here the appropriate fix? It sounds like we are adding an unnecessary EOF just for the sake of triggering some actions. could there be a cleaner way to handle this? I'm not familiar with the code base but by thinking aloud, is this possible: do not allow transaction/script/contract view switch until language server is fully restarted and having the latest information?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not really about the latest state of the language server, but rather about the way to ask language server to run the check again. I couldn't find a way to trigger re-check manually and not sure it actually exposed on Monaco editor 🤷‍♂️

And we don't add any footprint to the file, as we remove new line character after a small delay, which in turn triggers recheck.

@MaxStalker MaxStalker merged commit 9ab3167 into staging Apr 26, 2022
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.

Postmortem - Language Server Refactoring
3 participants