flip-fest icon indicating copy to clipboard operation
flip-fest copied to clipboard

VS Code Extension: Architecture improvements

Open srinjoyc opened this issue 4 years ago • 19 comments

👋   If you are interested in working on this issue, please check out the Getting Started guide on HackerEarth!

Description (Problem Statement)

The VS Code extension architecture could be improved so the extension becomes lighter and only handles the communication with the language server and the UI. This simplification would allow easier testing, less complexity and, better UX during the installation process. Currently, the extension requires the Flow CLI pre-installed in order to start and manage the emulator as well as to start the language server itself.

The proposed improved architecture would be for the language server to wrap the emulator by including flowkit dependency and start and manage the emulator state from there. The language server would then need to stream the emulator output to the extension for a developer to see in the output. The language server should also be bundled together with the extension as an npm module so it can be started directly from the VS Code.

Experience Required

  • Experience with Go
  • Experience with typescript/javascript
  • Experience with writing tests

Minimum Feature Set (Acceptance Criteria)

  • The CLI is no longer required for the VS Code extension to function
  • The language server handles emulator state
  • The emulator log is streamed to the extension and displayed in the output
  • The language server comes bundled together with the extension and the installation process doesn't have any additional steps
  • The functionality of the VS Code extension stays the same or is extended

Extension (Optional) Feature Set

  • Tests are improved to provide code coverage > 70% and are mostly consisting of unit tests
  • End to end tests are expanded to test all interactions
  • Implemented a release pipeline for new versions using Github actions
  • Implement code coverage reporting with Codecov (look at the CLI repo)

Milestone Requirements

  1. Document the basic architecture for the extension improvements
  2. Remove the CLI dependency from the extension and manage the emulator in the language server
  3. Refactor extension codebase so it becomes a lightweight client
  4. Make sure the release pipeline and tests are implemented

Software Requirements

Maintainability

  • Code architecture should provide simplicity and a minimalistic codebase that is easy to understand and extend.
  • The release process should automate releasing new versions to the marketplace and building all the dependencies.

Testing

  • Tests should have good coverage and allow for future changes with confidence.
  • End to end test with Cypress should test all interactions with the extension.

Other Requirements

Code standards

  • Follow code standards and requirements already in place in the existing repository.

Test coverage

  • Don't decrease code coverage.

Judging Criteria

Resources

srinjoyc avatar Sep 07 '21 11:09 srinjoyc

Hi 👋 my name is Gregor, and as a software engineer working on developer tools, I will be your point of contact for this task, so good luck, and don't hesitate to reach out to me with any problems or questions.

I'll keep an eye on your progress and will be reviewing your code.

You can comment here or find me on Discord (I'm sideninja#1970). Join the Flow Discord server if you're not there already!

devbugging avatar Sep 16 '21 08:09 devbugging

Hello, team Koala will be taking this one on. https://www.hackerearth.com/challenges/hackathon/flip-fest/dashboard/5801ad5/ @eburnette

eburnette avatar Sep 18 '21 01:09 eburnette

I see a possible problem with the problem description because it would lead to the language server and emulator being installed twice, once with the extension and once with the cli. It would be unfortunate if you ran something in vscode and it worked, but then you ran it from the cli and it didn't work, because the versions were different. Why not move the language server out of the cli, and have it run cli commands to do things (e.g., run the emulator). Installing the extension would install the language server and the cli as needed, and vscode would recognize when things need to be updated like it does now with go and go tools.

eburnette avatar Sep 18 '21 02:09 eburnette

The proposed architectural change is for the extension to become a minimalistic and light client that is there to only allow interactions with the developers and forward the command to the language server. Langauge server should handle the execution of those commands, which include execution of commands by using an emulator as a dependency (not as a separate process). You can see how that is possible in the CLI here: https://github.com/onflow/flow-cli/blob/master/pkg/flowkit/gateway/emulator.go but even further the language server can just use the flowkit package to execute all the commands it needs. This way the language server includes the emulator and it won't be a requirement for a developer to preinstall and handle the emulator state. Regarding versioning differences, the vscode extension needs to warn a developer if a new version is available the same way as the CLI does and the emulator. The CLI will also be stripped of the language server and vscode stuff as it doesn't have to include it after this change. Let me know if you need further explanation of the architecture.

devbugging avatar Sep 18 '21 13:09 devbugging

I had to make a number of changes to the vscode-cadence extension just to get it to build and debug for me. If you could review and merge those, it would be very helpful. https://github.com/onflow/vscode-cadence/pull/67

eburnette avatar Sep 19 '21 17:09 eburnette

I'd like to make a change to the requirements, which currently say the emulator has to be managed by the language server. Execution isn't normally a function of VSCode language servers. For example, the go language server doesn't start and stop go programs. So I'm currently leaning towards keeping the emulator and language server separate. They would both be stand-alone go programs, invoked by the extension without the CLI. We could embed the binaries into the extension or download them at install time. I'm leaning towards embedding them for simplicity, but either way would work. Let me know if this requirement change would be acceptable.

eburnette avatar Oct 24 '21 14:10 eburnette

Unfortunately the 'emulator' command in onflow/emulator doesn't have the right configuration and fails with an invalid signature message. So I'll have to make a new one that configures itself like flow-cli (looks for flow.json for example) but is a stand-alone program.

eburnette avatar Oct 24 '21 19:10 eburnette

Ok, I have to admit I need a pointer on how to make this work. I'm getting errors like this when I invoke the emulator

WARN[0001] ERR [fc27a3] [Error Code: 1006] invalid proposal key: public key 0 on account f8d6e0586b0a20c7 does not have a valid signature: [Error Code: 1009] invalid envelope key: public key 0 on account f8d6e0586b0a20c7 does not have a valid signature: signature is not valid

and in the extension, the "await ext.api.initAccountManager()" call in commands.ts doesn't return. It worked fine when calling "flow emulator" but not either of my "emulator" stand-alone commands.

eburnette avatar Oct 25 '21 01:10 eburnette

I'd like to make a change to the requirements, which currently say the emulator has to be managed by the language server. Execution isn't normally a function of VSCode language servers. For example, the go language server doesn't start and stop go programs. So I'm currently leaning towards keeping the emulator and language server separate. They would both be stand-alone go programs, invoked by the extension without the CLI. We could embed the binaries into the extension or download them at install time. I'm leaning towards embedding them for simplicity, but either way would work. Let me know if this requirement change would be acceptable.

I think a big advantage of implementing communication with an emulator in the LS is that any extension will then just have to implement a simple API instead of implementing a complex logic handling emulator state. There are other extensions being developed and I believe it would be better to avoid code duplication on the "frontend".

Ok, I have to admit I need a pointer on how to make this work. I'm getting errors like this when I invoke the emulator

This error seems like you are not passing correct env variables to the emaultor. You should make sure to pass private and public key to the emulator as described here: https://github.com/onflow/flow-emulator

devbugging avatar Oct 25 '21 08:10 devbugging

So basically the VSCode extension needs to pass through commands, like the one to start the emulator, to the language server process. This won't work in the long term for debugging but I'll see what I can do. What other extensions are being developed?

eburnette avatar Oct 25 '21 20:10 eburnette

This won't work in the long term for debugging

What type of debugging do you have in mind? and why do you think that will be a problem or how do you see the extension managing emulator would change that? I believe extensions should just be lightweight clients handling user interactions and LS should handle more complex operations and managing state.

What other extensions are being developed?

The other I know of is for Jetbrains IDE.

devbugging avatar Oct 26 '21 15:10 devbugging

What type of debugging do you have in mind?

For example, setting a breakpoint in a contract and stepping through it, looking at variables. The Language Server Protocol does not have any standard hooks for that. It's intended for use by an editor; running and debugging have to go through a non-standard interface.

eburnette avatar Oct 27 '21 00:10 eburnette

I don't think this would outweigh the pros of having the LS manage the emulator, firstly debugging is not possible and if it will be possible on the emulator we are not yet sure how is that gonna be implemented, secondly if the extension can detect interactions with the debugger it can communicate with LS to process commands for stepping and breakpoints (not sure why you think that wouldn't be possible) and also again implementing that on the extension will lead to each extension implementing debugging logic for emulator.

devbugging avatar Oct 27 '21 12:10 devbugging

Just trying to think ahead a bit but I don't want to confuse the issue. LSP is not general-purpose but we can work within its confines for now.

eburnette avatar Oct 27 '21 14:10 eburnette

I'm getting a null pointer crash in the languageserver in state.go because p.readerWriter is nil, is this a known problem?

func (p *State) ReadFile(source string) ([]byte, error) {
	return p.readerWriter.ReadFile(source)
}

eburnette avatar Oct 31 '21 00:10 eburnette

I'm sorry, but I just ran out of time on this one due to work projects. I got the cli dependency out, and the language server maintains the emulator state, but the log is not streamed, the server isn't bundled or installed (just hard-coded right now), and the emulator is started with a command, not flowkit.

eburnette avatar Oct 31 '21 02:10 eburnette

I'm sorry, but I just ran out of time on this one due to work projects. I got the cli dependency out, and the language server maintains the emulator state, but the log is not streamed, the server isn't bundled or installed (just hard-coded right now), and the emulator is started with a command, not flowkit.

No worries, I understand. You are still welcome to keep on working on this, also there are some participation awards to which you might be eligible.

devbugging avatar Nov 02 '21 09:11 devbugging

Good day @eburnette!

Thanks so much for all your hardwork & participation. In order to finalize winners & prepare for prize payout, we'll need the following actions from your end.

Please provide the following information by Nov 17, 2021, (in this GH Issue is fine):

1. Team Information

  • Team Members Information - Github Username + Email Contact + Percentage of prize allocation (total should = 100%)
  • All mentioned members MUST react to the post with a 👍 which will act as confirmation that the information is correct, or a 👎 to indicate that the information is not correct.
  • We will be reaching out via e-mail

🎖IMPORTANT: We will only proceed with prize payouts once all members have confirmed with 👍 on the post.

2. Video Demo (optional)

  • Please provide a 5-minute video demo to be featured & showcased in the FLIP Fest Closing Ceremonies
  • Link format & Downloadable (eg. Google Drive, Vimeo)
  • Content Format (Problem Statement, your work / how you solved it, final outcome)

We will be hosting Closing Ceremonies on November 23rd, 8AM PT where we'll having closing remarks from Dete & will be announcing the winners! I'll share the details here before Nov 17.

kimcodeashian avatar Nov 12 '21 23:11 kimcodeashian

Hey folks,

We've received and reviewed over 82 submissions! What an amazing community on Flow! To commemorate all the hard work done, we have finalized winners and will be announcing them during our Closing Ceremony on Nov 23rd, 8AM PT. Be sure to join us - there may be some attendance prizes & a keynote from our CTO, Dete 😉!

RSVP here so you don't miss out! See you then!

kimcodeashian avatar Nov 17 '21 01:11 kimcodeashian