[WIP] Enable web extension for Foam #1290

pderaaij · 2023-09-23T13:58:05Z

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.

pderaaij · 2023-09-25T18:34:50Z

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 vitest. However, this is not an easy ride as well. Vitest works well for unit tests, but integration tests are the problem. For reference, we need to run the test suite programmatically. We need to start the vscode environment and then run the test suite (see: run-tests/ts). Support for programmatically is limited in vitest (https://vitest.dev/advanced/api.html). However, this only works in ESM mode. Vscode, due to electron, runs only commonjs (microsoft/vscode#130367).

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 vitest is not the way forward. However, I did learn a thing or two. I am going to have a fresh look at getting mocha working. But, fresh ideas are welcome.

riccardoferretti

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.

riccardoferretti · 2023-09-26T09:28:29Z

packages/foam-vscode/src/features/preview/wikilink-embed.ts

@@ -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();


I think vsWorkspace.fs.readFile returns a promise, this was to me the only open question as to how to transition in the code

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?

This is still open, you can find all the info and continue the conversation here: #1295

pderaaij · 2023-10-04T05:13:33Z

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 expect. A library that's not easily replaced by chai or a other browser compatible assertion library. So, my conclusion is that if we really want to have tests for both extension and browser we would need to overhaul the current testing library, or maintain two libraries. Both are not preferable.

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?

riccardoferretti · 2023-10-05T09:07:10Z

Thanks @pderaaij for the investigation.

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 [...] 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.

Mmmm.. can you share a couple of VS Code extensions that use that approach?
I fear it might be a little brittle, but maybe I am missing something?
I guess testing the activation would validate that imports and packages are valid, but wouldn't cover the actual runtime.. then when an issue arises we could cover it with an explicit test (which requires a new assertion library, see comment below).
I am trying to figure out how big the window for uncaught bugs is with this approach..

Our assertion library is jest's expect. A library that's not easily replaced by chai or a other browser compatible assertion library. So, my conclusion is that if we really want to have tests for both extension and browser we would need to overhaul the current testing library, or maintain two libraries. Both are not preferable.

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.
I found some articles online of people that have done such thing, so at least we know it's possible.

pderaaij · 2023-10-10T18:05:31Z

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.

riccardoferretti · 2023-10-11T09:33:55Z

Ok, this is something to think about, I am still debating if it's "good enough" (it might be).
Can you get the PR in a place where all tests pass using jest? That feels like the first thing to ensure regardless.

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.

flancian · 2024-01-03T22:40:20Z

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?

pderaaij · 2024-07-08T20:20:17Z

@riccardoferretti this would be enough to make the web version work. However, note-embedding will not work correctly. I might be able to get it working via: microsoft/vscode#174080 (comment).

Additionally, I have not been able to get a simple end-2-end test working. My idea was to simply test if the extension was loaded. But I can't get mocha to play along.

Nevertheless, we can publish the extension for web with these comments and start to collect the feedback.

pderaaij · 2024-07-10T18:10:26Z

It took some time, but I got an end-2-end test working. It is a simple check that asserts if the foam extension is loaded.

@riccardoferretti I've added an extra CI step to build and verify the web version is working. If you see this differently, let me know.

My goal would be to get this PR merged. The next step is to attempt to change the markdown preview.

riccardoferretti

Thanks @pderaaij I have left a few comments, let me know what you think

riccardoferretti · 2024-07-10T18:26:16Z

packages/foam-vscode/web/webpack.config.js

+    mainFields: ['browser', 'module', 'main'],
+    extensions: ['.ts', '.js'], // support ts-files and js-files
+    alias: {
+      // Overwrite wikilink embedding as it will not work in the current setup on web


2 comments:

I wonder if we can handle it from within wikilink-embed.ts so that it's more explicit (I don't love webpack nor its black magic ;) )

if approach n.1 is not good, I would actually have /src/web/features/preview/wikilink-embed.ts -> src/features/preview/wikilink-embed-for-web-extension.ts (and add a comment on both files to explain what's happening)

First won't be possible. As the file contains an import of fs it will try to pack that. By overwriting the file on compile time, I am preventing these issues.

riccardoferretti · 2024-07-10T18:27:41Z

packages/foam-vscode/tsconfig.json

@@ -7,7 +7,8 @@
    "lib": ["ES2019", "es2020.string", "DOM"],
    "sourceMap": true,
    "strict": false,
-    "downlevelIteration": true
+    "downlevelIteration": true,
+    "skipLibCheck": true


what's the goal of this setting?

Preventing tsc to check the types within the project. I've added this to prevent the collisions between mocha and jest. We use mocha for the e2e tests and jest for the vscode extension. They are incompatible with each other, but for our use case that's not a problem. However, tsc scans the entire node_modules folder and detects the collisions.

riccardoferretti · 2024-07-10T18:30:05Z

packages/foam-vscode/src/web/features/preview/wikilink-embed.ts

+  workspace: FoamWorkspace,
+  parser: ResourceParser
+) => {
+  return md;


I would actually return something along the lines of:

return `<div class="foam-no-embeds-warning">Embeds are not supported in web extension: ${wikilink}</div>`;

foam-no-embeds-warning could be along the lines of foam-cyclic-link-warning

This is gonna save us from a bunch of bug reports about the missing feature ;)

riccardoferretti · 2024-07-10T18:31:36Z

packages/foam-vscode/src/test/web/web-extension.webtest.ts

+  vscode.window.showInformationMessage('Start all tests.');
+
+  test('Foam extension is loaded', () => {
+    assert.ok(vscode.extensions.getExtension('foam.foam-vscode'));


I like the idea of doing just a quick smoke check here, making sure that:

the extension is present

the API is exposed

a few commands are available

I think that should be enough to get started

I am stuck on expanding the tests. See microsoft/vscode-test-web#130.

Once this is solved, I'll update the PR.

packages/foam-vscode/src/test/web/index.ts

riccardoferretti · 2024-07-10T18:33:34Z

packages/foam-vscode/src/services/editor.ts

@@ -1,6 +1,5 @@
 import { isEmpty } from 'lodash';
 import { asAbsoluteUri, URI } from '../core/model/uri';
-import { TextEncoder } from 'util';


doesn't remove these import affect the native extension?

Not required anymore, as it is default part from node > 16.

package.json

riccardoferretti · 2024-07-13T16:02:49Z

Hi @pderaaij I was trying to test this locally, but I am getting the following error in the browser

Sequence of steps:

checkout the branch
yarn reset
cd packages/foam-vscode
yarn compile-web
yarn package-web
yarn open-in-browser
Open foam-template
go to getting-started.md (so far it is working as the links showing up in the "All Links" panel are correct)
hover with the mouse on a link (interesting that the navigation still work)

Also, the Foam: Show Graph command doesn't work (seems related to the lookup of the webview).

Can you confirm the issues?

pderaaij · 2024-07-13T17:26:08Z

Yes, the Buffer one I found out as well. It is locally in my stage, but yet to commit. Will do so.

It is a webpack config issue by the way. Easily resolved.

pderaaij · 2024-07-14T18:26:27Z

Got the tests working:

Functionality should be working now. Let me know what you think @riccardoferretti

riccardoferretti · 2024-08-08T20:37:15Z

First of all, amazing work @pderaaij! This is not a simple change!

There is one thing that I am really hesitating on, and that's mocha. I normally wouldn't like to have 2 "competing" libraries, in this specific case the fact that it's for testing and the extra magic on the TS front makes me even more wary.

So, I am looking at the code and am trying to understand where jest falls short.
Why can't packages/foam-vscode/src/test/web/index.ts use jest? I understand the limitations on the assertion library, and in that regard I would suggest we don't need an assertion library for two tests - if we can run them via jest.
Can you help me understand?

Thanks!

pderaaij · 2024-08-09T07:49:54Z

As far as I understand the problem is that Jest is an unit testing library. It is designed to run within the nodejs environment, but not the browser.

The test suite runs in the browser. The vscode-web-test extension starts a local VS code web environment in which Foam is loaded. After activation the test suite runs in the browser. For this we need a browser library. `` mocha`. Is the suggested library by the vscode docs.

I know it isn't ideal, but it is the pattern I saw in many repositories.

riccardoferretti · 2024-08-10T17:46:40Z

I see what you mean.
I was testing the extension and I am getting an error when showing the node graph, are you having the same issue?

pderaaij · 2024-08-12T19:21:53Z

@riccardoferretti Quite sure it was working, but can't understand how it could be working. I worked on a concept to make the graph working again. Could you verify if this works for your? run npm run compile-web before running npm run open-in-web

riccardoferretti · 2024-09-16T21:34:27Z

Thanks @pderaaij - I think things are really coming together and I was dealing with the odd sensation that there is something that rubs me the wrong way.

After much thinking I think I have come to the source of the issue: I feel too many bits are impacted, and between webpack and mocha, and the extra TS settings, I am very hesitant.
The work you have done is amazing, and I think I found a solution that I am comfortable with.
I have created a PR (#1395) that leverages the great parts of this one, but without introducing webpack/mocha/etc with some compromise (e.g. no web tests).

More info on the PR itself, please let me know what you think, and thanks again for making this possible!

@pderaaij

See #1290 for context. Major thanks to @pderaaij that did all the hard work here. * using js-sha1 instead of node's crypto to compute sha1 * Using esbuild to bundle native and web extension (WIP) * Added message regarding unsupported embeds in web extension * support for graph webview in web extension

riccardoferretti reviewed Sep 26, 2023

View reviewed changes

pderaaij mentioned this pull request Oct 4, 2023

[WIP] Enable support for vscode.dev #1127

Closed

riccardoferretti mentioned this pull request Jun 24, 2024

Add support for VSCode Web #749

Closed

pderaaij force-pushed the web-extension branch from 5c3b2e6 to 2144abf Compare July 8, 2024 17:39

pderaaij force-pushed the web-extension branch from be54489 to b1d05cc Compare July 10, 2024 18:12

riccardoferretti reviewed Jul 10, 2024

View reviewed changes

pderaaij added 11 commits July 14, 2024 20:04

Migration steps towards web extension

9cf8d17

Migration steps towards web extension

d9f6a90

Replace direct use of fs by vscode.workspace

0a79d9e

Add polyfills and correct configuration so extension can be compiled

5f1cff2

Enable local test environment for web extension

95619e8

Failed attempt to get web e2e tests running

fa6a880

Refresh PR to make web version working

eea0e3b

Make web working, without interference of non-working web tests

eb4cac0

Fix compilation of web bundle

c34eb45

Add an e2e test for web and CI build step

28805a9

Rework test suite and webpack config

53f821e

pderaaij force-pushed the web-extension branch from 41168c3 to 53f821e Compare July 14, 2024 18:15

Make graph working again

68a1cda

pderaaij mentioned this pull request Aug 12, 2024

Exclude workspace when linking to a file #1372

Merged

riccardoferretti mentioned this pull request Sep 16, 2024

Foam Web Extension #1395

Merged

pderaaij closed this Sep 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Enable web extension for Foam #1290

[WIP] Enable web extension for Foam #1290

pderaaij commented Sep 23, 2023

pderaaij commented Sep 25, 2023

riccardoferretti left a comment

riccardoferretti Sep 26, 2023

tegkhanna Nov 7, 2023

riccardoferretti Nov 7, 2023 •

edited

Loading

pderaaij commented Oct 4, 2023

riccardoferretti commented Oct 5, 2023

pderaaij commented Oct 10, 2023

riccardoferretti commented Oct 11, 2023

flancian commented Jan 3, 2024

pderaaij commented Jul 8, 2024

pderaaij commented Jul 10, 2024

riccardoferretti left a comment

riccardoferretti Jul 10, 2024

pderaaij Jul 10, 2024

riccardoferretti Jul 10, 2024

pderaaij Jul 10, 2024

riccardoferretti Jul 10, 2024

pderaaij Jul 11, 2024

riccardoferretti Jul 10, 2024

pderaaij Jul 11, 2024 •

edited

Loading

riccardoferretti Jul 10, 2024

pderaaij Jul 11, 2024 •

edited

Loading

riccardoferretti commented Jul 13, 2024

pderaaij commented Jul 13, 2024

pderaaij commented Jul 14, 2024

riccardoferretti commented Aug 8, 2024

pderaaij commented Aug 9, 2024

riccardoferretti commented Aug 10, 2024

pderaaij commented Aug 12, 2024

riccardoferretti commented Sep 16, 2024

[WIP] Enable web extension for Foam #1290

[WIP] Enable web extension for Foam #1290

Conversation

pderaaij commented Sep 23, 2023

pderaaij commented Sep 25, 2023

riccardoferretti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

riccardoferretti Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

pderaaij commented Oct 4, 2023

riccardoferretti commented Oct 5, 2023

pderaaij commented Oct 10, 2023

riccardoferretti commented Oct 11, 2023

flancian commented Jan 3, 2024

pderaaij commented Jul 8, 2024

pderaaij commented Jul 10, 2024

riccardoferretti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pderaaij Jul 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pderaaij Jul 11, 2024 • edited Loading

Choose a reason for hiding this comment

riccardoferretti commented Jul 13, 2024

pderaaij commented Jul 13, 2024

pderaaij commented Jul 14, 2024

riccardoferretti commented Aug 8, 2024

pderaaij commented Aug 9, 2024

riccardoferretti commented Aug 10, 2024

pderaaij commented Aug 12, 2024

riccardoferretti commented Sep 16, 2024

riccardoferretti Nov 7, 2023 •

edited

Loading

pderaaij Jul 11, 2024 •

edited

Loading

pderaaij Jul 11, 2024 •

edited

Loading