Virtual File Systems & Shopify Liquid VS Code extension for Web #529

charlespwd · 2024-10-08T18:25:10Z

What are you adding in this PR?

Major breaking changes in most packages (mostly internal APIs)
- vscode-extension:
  - We now have a browser entry in our package.json
  - The src folder has been split into node, common and browser folders
  - The browser module starts the language server in a Web Worker
  - The node module starts the language server in a subprocess
  - Dropped support for CLI based language servers (legacy extension)
  - There's two new ways to test/debug the VS Code extension in browser contexts (see tophatting section below).
- theme-check-*, and theme-language-server-*:
  - Replace AbsolutePath concerns with Uris.
    - Absolute paths don't make sense in browser
    - Absolute paths don't make sense with remote files
- Replace all "fs-based" Dependencies with a single fs: AbstractFileSystem dependency
  - The old injections are now common implementations based on the abstract fs
  - For the CLI theme check, a NodeFileSystem is available (and used by default)
  - For the CLI language server, a NodeFileSystem is available (and used by default)
  - For the VS Code extension, a VsCodeFileSystem is available
    - This file system implementation queries the VS Code FileSystem API (only available in the extension host thread) for file contents. This query is done on the language server via non-standard LSP messages:
      - fs/readFile -> a TextDecoded vscode.workspace.fs.readFile(uri)
      - fs/readDirectory -> vscode.workspace.fs.readDirectory(uri)
      - fs/stat -> vscode.workspace.fs.stat(uri)
    - In node, the same happens but short circuits to the NodeFileSystem for file:/// URIs.
      - We don't need to send file contents over STDIO when we can query the FS directly
- The codemirror-language-client's playground implements a mock "MainThreadFileSystem" that we could use as inspiration for online-store-web

Demo

vscode-for-web-vfs.mp4

Top-hatting

There's 2 new ways you can do this. They are documented in docs/contributing. But here's a video.

Debugging in Chromium VS Code for Web

The "true" E2E way. Demo worthy version of "showing it works in browser"

vscode-for-web-vfs-2.mp4

Debugging in the Desktop app

A faster, debuggable way that runs the browser extension in the desktop app. This disables all the Node APIs but it lets you debug the code.

vscode-for-web-vfs-desktop.mp4

Debugging the CodeMirror Language client

(Not new, but you should know that this flow is also updated to virtual file systems).

vscode-for-web-vfs-codemirror.mp4

What's next? Any followup issues?

Make loadConfig work in-browser
Make the prettier plugin work in-browser

What did you learn?

A couple of neat things that the vscode libraries already offer.
- The web worker stuff is offered via BrowserMessageReader(worker) and BrowserMessageWriter(worker).
- vscode.workspace.fs is pretty cool. It does a pretty clever the URIScheme -> FileSystemProvider mapping that makes it so we could eventually have a "ShopifyBackendFileSystem" that would write directly to the storefront.

Before you deploy

I included a bunch major bump changeset

charlespwd · 2024-10-08T18:26:45Z

packages/theme-language-server-browser/src/index.ts

-  };
-
+  const reader = new BrowserMessageReader(worker);
+  const writer = new BrowserMessageWriter(worker);


I learned by looking at examples that there was an easier way :)

charlespwd · 2024-10-08T18:27:17Z

packages/vscode-extension/package.json

@@ -40,6 +40,7 @@
    "dev:build": "webpack --mode development",
    "dev:syntax": "yarn --cwd ./syntaxes dev",
    "dev:watch": "webpack --mode development --watch",
+    "dev:web": "yarn build && vscode-test-web --permission=clipboard-read --permission=clipboard-write --browserType=chromium --extensionDevelopmentPath=.",


vscode-test-web is the one that lets you run chrome and have vscode run in it with your extension loaded. It was a real breeze ngl!

charlespwd · 2024-10-08T18:27:45Z

packages/vscode-extension/package.json

@@ -83,7 +85,8 @@
  "activationEvents": [
    "workspaceContains:**/.theme-check.yml"
  ],
-  "main": "./dist/extension.js",
+  "main": "./dist/node/extension.js",
+  "browser": "./dist/browser/extension.js",


Major difference is here: We have a different build for web that uses theme-language-server-browser instead of theme-language-server-node

charlespwd · 2024-10-08T18:28:18Z

packages/vscode-extension/package.json

+            "messages",
+            "verbose"
+          ]
+        },


Added that stuff in there because I was tired of not having code completion in settings.json to enable the LSP logging

packages/vscode-extension/src/browser/extension.ts

packages/vscode-extension/src/browser/server.ts

packages/vscode-extension/src/node/extension.ts

charlespwd · 2024-10-17T15:53:47Z

packages/theme-check-common/src/AbstractFileSystem.ts

+/**
+ * The AbstractFileSystem interface is used to abstract file system operations.
+ *
+ * This way, the Theme Check library can be used in different environments,
+ * such as the browser, node.js or VS Code (which works with local files, remote
+ * files and on the web)
+ */
+export interface AbstractFileSystem {
+  stat(uri: string): Promise<FileStat>;
+  readFile(uri: string): Promise<string>;
+  readDirectory(uri: string): Promise<FileTuple[]>;
+}
+
+export enum FileType {
+  Unknown = 0,
+  File = 1,
+  Directory = 2,
+  SymbolicLink = 64,
+}
+
+export interface FileStat {
+  type: FileType;
+  size: number;
+}
+
+/** A vscode-uri string */
+export type UriString = string;
+
+export type FileTuple = [uri: UriString, fileType: FileType];


Probably the most important file in this large diff.

charlespwd · 2024-10-17T15:56:26Z

packages/theme-language-server-common/src/server/startServer.ts

-    fileExists,
-    fileSize,
-    filesForURI,
-    findRootURI: findConfigurationRootURI,
-    getDefaultLocaleFactory,
-    getDefaultTranslationsFactory,
-    getDefaultSchemaLocaleFactory,
-    getDefaultSchemaTranslationsFactory,
-    getThemeSettingsSchemaForRootURI,


charlespwd · 2024-10-17T15:56:42Z

packages/theme-language-server-common/src/server/startServer.ts

-      fileExists,
-      fileSize,
-      findRootURI: findConfigurationRootURI,
-      getDefaultLocaleFactory,
-      getDefaultTranslationsFactory,
-      getDefaultSchemaLocaleFactory,
-      getDefaultSchemaTranslationsFactory,
+      fs,


charlespwd · 2024-10-17T15:58:18Z

packages/theme-language-server-common/src/types.ts

@@ -48,7 +51,7 @@ export interface RequiredDependencies {
   * @param uri - a file path
   * @returns {Promise<Config>}
   */
-  loadConfig(uri: URI): Promise<Config>;
+  loadConfig(uri: URI, fileExists: (uri: string) => Promise<boolean>): Promise<Config>;


the fileExists here is a performance improvement. Wanted loadConfig to use findRoot and that thing depends on fileExists which should be the CachedFileSystem, not the raw one.

charlespwd · 2024-10-17T15:59:44Z

packages/vscode-extension/src/node/extension.ts

-      { scheme: 'file', language: 'liquid' },
-      { scheme: 'file', language: 'plaintext' },
-      { scheme: 'file', language: 'html' },
-      { scheme: 'file', language: 'javascript' },
-      { scheme: 'file', language: 'css' },
-      { scheme: 'file', language: 'scss' },
-      { scheme: 'file', language: 'json' },
-      { scheme: 'file', language: 'jsonc' },
+      { language: 'liquid' },
+      { language: 'plaintext' },
+      { language: 'html' },
+      { language: 'javascript' },
+      { language: 'css' },
+      { language: 'scss' },
+      { language: 'json' },
+      { language: 'jsonc' },


This thing took me a while to figure out since I wrote this 3 years ago. We need to remove scheme in order for completion requests from vscode-vfs:// URIs to trigger.

This comes with the drawback of showing Document Link errors in git:// URIs. (which I believe are git diff URIs)...

charlespwd · 2024-10-17T16:01:23Z

packages/vscode-extension/webpack.config.js

+      libraryTarget: 'var',
+      library: 'serverExportVar',
+      devtoolModuleFilenameTemplate: '../../[resource-path]',


The only reason I need 2x a config that should look like one is because of this difference here. The worker has to be "import-less", but the extension needs a require('vscode') call in the output that gets shimmed.

We still have an optiomal dependency that is otherwise implemented by a naive implementation (for CLI use cases), but in the language server we prefer the buffer over the file in the file system... Which makes me think we could get rid of that by actually making that logic part of the getTranslationsFactory directly. Prefer theme files over fs.

All that logic didn't need to be in the language server. This makes the dep injection easier too because we no longer have this upstream concern.

Replace with AbstractFileSystem-based implementation

Replace with abstract-file-system-based implementation TODO make that shit faster. Seems highlighy unoptimized.

Replace with abstract-file-system-based implementation in common code.

- Adds support for VS Code for Web - Adds support for remote files

You don't need to query VS Code _every time_ the user types something for the same imformation. That's rather slow! We'll invalidate the relevant caches when files are saved/deleted/created/renamed.

Made to demo what it's like to use a VirtualFileSystem in a non-vscode environment.

charlespwd · 2024-10-23T19:39:39Z

packages/vscode-extension/src/browser/extension.ts

+  client.onRequest('fs/readDirectory', async (uriString: string): Promise<FileTuple[]> => {
+    const results = await workspace.fs.readDirectory(Uri.parse(uriString));
+    return results.map(([name, type]) => [path.join(uriString, name), type]);
+  });
+
+  const textDecoder = new TextDecoder();
+  client.onRequest('fs/readFile', async (uriString: string): Promise<string> => {
+    const bytes = await workspace.fs.readFile(Uri.parse(uriString));
+    return textDecoder.decode(bytes);
+  });
+
+  client.onRequest('fs/stat', async (uriString: string): Promise<FileStat> => {
+    return workspace.fs.stat(Uri.parse(uriString));
+  });


Probably should have highlighted this code in the pairing session as well. This is the client code that handles the VsCodeFileSystem LSP requests. @aswamy @graygilmore

This is the one that calls the VS Code FileSystem API

vscode.workspace.fs.readFile() // returns a UInt8Array (bytes) // AbstractFileSystem.readFile is Promise<string>, this is why we do textDecoder.decode(bytes) on the result.

packages/theme-language-server-common/src/diagnostics/runChecks.ts

I also removed the double reporting of parser errors. Now that theme check is on the same parser as the prettier plugin, there's no point in showing the error message reported from two different sources. Fixes #533

aswamy

Sorry took some time, but we should be good. Minor comments + concerns, but nothing blocking atm.

Tophat complete:

Able to run VScode web, able to edit files, save it
Able to run VScode web in "sandbox" mode (vscode-test-web), able to edit files - does not persist file as expected
Able to run desktop VSCode with web extension, able to edit files, save it
Able to run Codemirror Client, able to edit files, save it

⚠️ Noticed that when you open any files in the repo, there is a weird error where we look for .theme-check.yml file in the wrong directory. It could be one of the rootPath functions are messed up because i saw this in all VScode scenarios above. However, Node extension had it as a thrown error, while Browser Extension had it as a info-message.

aswamy · 2024-10-24T15:12:56Z

package.json

@@ -23,6 +23,7 @@
    "build": "yarn workspaces run build",
    "build:ci": "yarn workspaces run build:ci",
    "build:ts": "yarn workspaces run build:ts",
+    "dev:web": "yarn --cwd packages/vscode-extension dev:web",


glob package fails if you don't have node v20-22. Could we add an .nvmrc to have v22?

Durr I have a .nvmrc but I git ignored/excluded it? Yeah def should do

packages/vscode-extension/src/common/VsCodeFileSystem.ts

packages/vscode-extension/src/common/formatter.ts

packages/theme-language-server-common/src/translations.ts

.vscode/launch.json

packages/theme-language-server-common/src/documentLinks/DocumentLinksProvider.spec.ts

packages/theme-check-common/src/checks/translation-key-exists/index.spec.ts

packages/theme-check-node/src/temp.ts

packages/theme-check-common/src/test/MockFileSystem.spec.ts

packages/theme-check-common/src/context-utils.ts

charlespwd commented Oct 8, 2024

View reviewed changes

packages/vscode-extension/src/browser/extension.ts Outdated Show resolved Hide resolved

charlespwd commented Oct 8, 2024

View reviewed changes

packages/vscode-extension/src/browser/server.ts Outdated Show resolved Hide resolved

charlespwd commented Oct 8, 2024

View reviewed changes

packages/vscode-extension/src/node/extension.ts Outdated Show resolved Hide resolved

charlespwd force-pushed the feature/521-virtual-file-system branch 13 times, most recently from bdbbc56 to 35b07d1 Compare October 15, 2024 18:15

charlespwd force-pushed the feature/521-virtual-file-system branch 3 times, most recently from 7c5b372 to 71d5086 Compare October 16, 2024 19:50

charlespwd commented Oct 17, 2024

View reviewed changes

charlespwd force-pushed the feature/521-virtual-file-system branch from 3bc7078 to 3919133 Compare October 17, 2024 16:03

charlespwd added 12 commits October 18, 2024 14:38

Move buffer translations concern to theme-check-common

fc7cb9a

All that logic didn't need to be in the language server. This makes the dep injection easier too because we no longer have this upstream concern.

Remove filesForURI language server dependency

4dcc202

Replace with AbstractFileSystem-based implementation

Remove getThemeSettingsSchemaForURI injection

323ba07

Replace with abstract-file-system-based implementation TODO make that shit faster. Seems highlighy unoptimized.

Remove findRootURI injection

a016347

Replace with abstract-file-system-based implementation in common code.

Implement and setup basic VsCodeFileSystem

18c95dd

- Adds support for VS Code for Web - Adds support for remote files

Optimize FS calls in the Language Server

9840c50

You don't need to query VS Code _every time_ the user types something for the same imformation. That's rather slow! We'll invalidate the relevant caches when files are saved/deleted/created/renamed.

Implement MainThreadFileSystem in CodeMirror Playground

32c11f4

Made to demo what it's like to use a VirtualFileSystem in a non-vscode environment.

Add a bunch of changesets

81b6782

Docs changes and webpack config cleanup

f2f3c0e

Add toplevel yarn dev:web command

b77e59d

Minor cleanups

6e2b36e

charlespwd force-pushed the feature/521-virtual-file-system branch from 0a58241 to 6e2b36e Compare October 18, 2024 18:38

charlespwd mentioned this pull request Oct 18, 2024

Make the formatter work in the web extension #533

Closed

Add Debug Web Extension in Chromium launch config

a76b2b2

charlespwd commented Oct 23, 2024

View reviewed changes

packages/theme-language-server-common/src/diagnostics/runChecks.ts Outdated Show resolved Hide resolved

Add support for prettier formatting in web/remote contexts

56d92fb

I also removed the double reporting of parser errors. Now that theme check is on the same parser as the prettier plugin, there's no point in showing the error message reported from two different sources. Fixes #533

aswamy approved these changes Oct 25, 2024

View reviewed changes

charlespwd mentioned this pull request Oct 25, 2024

Setup the prettier organize import plugin #540

Open

charlespwd added 2 commits October 25, 2024 10:22

PR Cleanups

1f173a1

All dev servers on :3000

6ef16d6

charlespwd force-pushed the feature/521-virtual-file-system branch from 7b951fc to 6ef16d6 Compare October 25, 2024 14:22

charlespwd merged commit 4b574c1 into main Oct 25, 2024
6 checks passed

charlespwd deleted the feature/521-virtual-file-system branch October 25, 2024 14:41

mgmanzella added #gsd:43130 Liquid DX and removed #gsd:43130 Liquid DX labels Oct 28, 2024

charlespwd added #gsd:43130 Liquid DX and removed #gsd:43130 Liquid DX labels Oct 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Virtual File Systems & Shopify Liquid VS Code extension for Web #529

Virtual File Systems & Shopify Liquid VS Code extension for Web #529

charlespwd commented Oct 8, 2024 •

edited

Loading

charlespwd Oct 8, 2024

charlespwd Oct 8, 2024

charlespwd Oct 8, 2024 •

edited

Loading

charlespwd Oct 8, 2024

charlespwd Oct 17, 2024

charlespwd Oct 17, 2024

charlespwd Oct 17, 2024

charlespwd Oct 17, 2024

charlespwd Oct 17, 2024

charlespwd Oct 25, 2024

charlespwd Oct 17, 2024

charlespwd Oct 23, 2024

charlespwd Oct 23, 2024

aswamy left a comment

aswamy Oct 24, 2024

charlespwd Oct 25, 2024

Virtual File Systems & Shopify Liquid VS Code extension for Web #529

Virtual File Systems & Shopify Liquid VS Code extension for Web #529

Conversation

charlespwd commented Oct 8, 2024 • edited Loading

What are you adding in this PR?

Demo

Top-hatting

Debugging in Chromium VS Code for Web

Debugging in the Desktop app

Debugging the CodeMirror Language client

What's next? Any followup issues?

What did you learn?

Before you deploy

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charlespwd Oct 8, 2024 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aswamy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charlespwd commented Oct 8, 2024 •

edited

Loading

charlespwd Oct 8, 2024 •

edited

Loading