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
[WIP] Enable web extension for Foam #1290
base: master
Are you sure you want to change the base?
Conversation
Update: The extension is running quite well in the online environment at first sight. To be sure - and for regression, I'd like to run the integration tests for web as well. This is where the pain begins. Last time I got stuck in a long loop to get Mocha working, so this time I went for There seems to be some ways to get commonjs working in vscode, but I am wondering if we want all these changes just to get testing for web working. For now, I feel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for giving it a second stab! I am happy to see that the code is basically ready for the transition, except for the readFileSync
which I still think needs a solution (have you checked if the toString
works?)
Unfortunately I am not super versed in mocha nor vitest, so can't help much on that front. The commonjs requirement is a bit of a limitation, and I agree that is probably better to play within that unless a solid solution can be found otherwise.
@@ -194,7 +193,7 @@ function contentExtractor( | |||
parser: ResourceParser, | |||
workspace: FoamWorkspace | |||
): string { | |||
let noteText = readFileSync(note.uri.toFsPath()).toString(); | |||
let noteText = vsWorkspace.fs.readFile(toVsCodeUri(note.uri)).toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think vsWorkspace.fs.readFile
returns a promise, this was to me the only open question as to how to transition in the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, is this still open? If yes can we get around by writing a new markdown-it
plugin to handle the async operation?
Their documentation has some guidelines. Though I don't follow the wording aptly but I can give it a quick stab by forking markdownItRegex
. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still open, you can find all the info and continue the conversation here: #1295
I've been working on getting the tests to work with Mocha. Although it seemed I was making progress I ran into a big problem. Our assertion library is jest's My suggestion is to enable the brwoser extension and only add a test suite that checks if Foam can be activated in the web browser. A pattern that I have seen at more VSCode extensions. This way we at least confirm the webpack bundle is okay and works. Specific checks for working functionality in the browser would not exist. Only perhaps if it is a recurring event in which we would write a specific regression test. @riccardoferretti what do you think? |
Thanks @pderaaij for the investigation.
Mmmm.. can you share a couple of VS Code extensions that use that approach?
I am not against migrating over time to another library that would be more web friendly. Basically all new and edited tests would use the new library until we reach a point of switch over. |
Annoyingly enough I can't find these examples anymore in my history and a quick GitHub search leaves me blank as well. I have researched so many repo's I might lost track how often I have seen this pattern. I do agree it feels quite brittle. We could first rewire the testing framework. Or we just publish the web extension and rewrite the tests based on incoming reports. But that's not the greatest experience. In any case, it will require some work. |
Ok, this is something to think about, I am still debating if it's "good enough" (it might be). I am open to your suggestion about using the legacy framework (jest) for the regular testing, and only have some smoke tests for the web extension. |
Ahoy there! I come with few skills but willing to contribute help to try to merge this if I can provide any. I also know several other people who are interested in getting Foam to work on VS Code web :) What's left to be done? Could the extension be merged as-is without interfering with code quality/stability for regular (non-browser-based) VS Code users, maybe with a warning/alpha disclaimer? |
I am making a fresh attempt on getting Foam to work on the browser. I'll close #1127 later on, but am keeping it for reference. So far, so good. But long way to go.