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

Improve VS Code Extension Contributing Guidelines #53

Closed
jsjoeio opened this issue Jul 4, 2020 · 5 comments · Fixed by #65
Closed

Improve VS Code Extension Contributing Guidelines #53

jsjoeio opened this issue Jul 4, 2020 · 5 comments · Fixed by #65
Labels
documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed

Comments

@jsjoeio
Copy link
Contributor

jsjoeio commented Jul 4, 2020

I wanted to give a stab at contributing but I got stuck getting this extension to work locally. I tried adding a new hello world command to get things started, but I'm struggling to get that working.

It would be awesome for us to add some docs for getting this working locally! A few thoughts:

  • when I run the extension locally (f5), do I need to to run it in a foam workspace and have the other extensions installed?
  • tips for writing tests?
@jevakallio jevakallio transferred this issue from foambubble/foam-vscode Jul 4, 2020
@jevakallio jevakallio changed the title Improve Contributing Guidelines Improve VS Code Extension Contributing Guidelines Jul 4, 2020
@jevakallio
Copy link
Collaborator

@jsjoeio thanks! I'll get to updating the contribution guidelines, but for the time being:

  • When you run the extension, it starts a new extension host instance of VS Code. In that instance, open a Foam workspace (e.g. your personal one, or a test-specific one created from foam-template). This is strictly not necessary, but the extension won't auto-run unless it's in a workspace with a .vscode/foam.json file, so you'd need to test it by manually running any command.

  • No idea for tests, never written one for VS Code. Will have to learn more about this, I'm assuming there are general resources for this.

Contributions welcome!

@jevakallio
Copy link
Collaborator

(Migrated issue from foam-vscode to foam monorepo)

@jevakallio jevakallio added enhancement New feature or request documentation Improvements or additions to documentation help wanted Extra attention is needed labels Jul 4, 2020
@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jul 5, 2020

It seems like now that the project is using a monorepo format, it's a bit less straightforward to get the extension up and running locally. Any chance you can provide some guidance on how to do that? (note: new to Yarn workspaces)

Here are some issues I've run into so far:

TypeScript compile errors

Compiling the foam-vscode project spits out errors unless I pass the --skipLibCheck flag (it's upset with the node_modules in the neighboring directories). Does that mean the tsconfig.json should be updated to exclude all node_modules so it doesn't look at the neighboring directories? Or am I missing something?

Running the extension

Before, I could hit F5 to run the new extension host instance of VS Code. Now, it doesn't seem to know that it's a VS Code extension, unless I hit F5 with the extension.ts file active in the workspace? And it was complaining saying engines is mandatory, so I had to add it to the root package.json.

Testing the extension in development

This ones may sound silly, but bare with me...but when I'm testing the extension locally in the new extension host instance of VS Code and I'm using a Foam workspace (i.e. my personal one) and I want to run a command:

  • How do I run the command for the local extension if that workspace also has the published version of the foam-vscode extension?

Does that make sense?

P.S. - if I get some help, I promise to help contribute back by turning this information into some docs! 😄

@jevakallio
Copy link
Collaborator

jevakallio commented Jul 5, 2020

@jsjoeio these are very good points you raise! I was a bit hasty with the monorepo move. Trying to move a bit too fast with the few free hours I have.

Running the extension

After migration I did not test running the foam-vscode project from the monorepo -- I opened a new VS Code instance from the packages/foam-vscode directory, and that worked as normal.

Those tasks are defined in the tasks.json and launch.json config files in foam-vscode/.vscode.

  • I added those to the workspace monorepo root in 3aa9664, so now the extension should start from both when running from the monorepo workspace, and the foam-vscode workspace
  • I added a root-level watch task so the standard vscode npm watch task can pick it up here: 57811a8

I did not get the engines is mandatory error -- can you check if this still persists?

TypeScript compile errors

I discovered the same issue, where the TS compiler was not happy with conflicting global types for assertion methods exported by mocha and jest.

I don't think --skipLibCheck is the way to go here, instead we should harmonize our types. Since we actually have real tests written in Jest in foam-workspace-manager but no real tests written for the vscode extension or cli with Mocha, I opted to remove the Mocha dependencies for now with the goal of replacing them with Jest.

More info in the commit message in 24d81a9

The project builds cleanly for me now. I'm not a TypeScript expert (team Flow 😢), so if anyone has ideas on best practices for setting up TypeScript monorepos, I'd be all ears.

Testing the extension in development

When you start the extension development host, the local version of the extension "shadows" or overrides any installed versions of the package with the same ID. So when you F5/start the extension host, you should always be running the local version, plus any other extensions your workspace/vscode is configured to run.

AFAIK you can't run the "shadowed" base install while running the local extension

Further monorepo work

There's a still a bit of work to do to make the monorepo setup smooth. Tracking it in #57

Documentation

P.S. - if I get some help, I promise to help contribute back by turning this information into some docs!

Yes please 🙏🙏🙏🙏🙏

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Jul 5, 2020

@jevakallio thanks for the detailed explanations and feedback! Seems to be working a lot smoother now! 😃

can you check if this still persists?

I am no longer seeing this.

I opted to remove the Mocha dependencies for now with the goal of replacing them with Jest

😍 So glad to hear this! The last extension I worked on, we did the same It made things a lot easier.

Sweet! Well I was able to get things running and submitted a PR to improve the docs. With that, others should be able to get up and running quickly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants