Notebook "Pure" Renderer API

[connor4312]

Problem

With native integration of notebooks into VS Code, we want to foster an ecosystem for notebooks in VS Code. One of the goals is to build notebook renderers that can be reused in other applications, such as Azure Data Studio.

Right now renderers are 'just another' VS Code extension, with full access to the VS Code API, file system, and so on. It is impractical for another consumer like ADS to implement the entire API to ensure that renderers work universally. Renderer authors are only likely to test on one platform, so it's important that the renders can be built in a small surface area that works reliably between products.

Approach

1st draft, work in progress, please give feedback!

Quick Review

You can take a look the structure of the renderer starter repo to get an idea of what building a renderer entails today.

Module Loading

Currently VS Code loads the extension from the main file given in the package.json, which in turn registers the renderer. We'd like renderers to be ship-able in extensions, but loading the main file will generally require presence of the vscode module/shim.

There are two paths that I'm thinking about here:

Path 1: ExtensionHost-like registration

There's precedent in adding extra entrypoints in the package.json, for example the browser entrypoint or de-facto module fields adopted by Webpack/Rollup/etc. Therefore an additional notebookRenderer field is a natural addition:

{
  "name": "My Cool Renderer",
  "main": "./out/extension.js",
+ "notebookRenderer": "./out/renderer.js"

The primary vscode API/import should not be accessible to notebook renderers. VS Code provide the require function available to renderers, so this is easy to enforce. I suggest moving renderer-only APIs to a separate types package, or a subpath like vscode/notebookRenderer.

Q: There are common classes like Events and Uris in these APIs. vscode-uri is already a standalone npm module, should we do the same thing for Events or other APIs?

Q: Should there be any other require imports allowed? Disallowing these will require bundling of the extension, which is good practice anyway, and makes it portable between the extension host and browsers.

The registration code in here would then be similar to that in the usual extension.js, except with fewer APIs, for instance:

./src/renderer.ts

import { RendererContext, registerNotebookOutputRenderer } from 'vscode/notebookRenderer';

export function activate(context: RendererContext /* != ExtensionContext */) {
  context.subscriptions.push(registerNotebookOutputRenderer(/* ... */));
}

Path 2: Package.json-only registration

Today we have a contributes section of the package.json that includes notebook renderers. We could add an additional property here, like entrypoint, which links directly to the UI/browser code for the renderer.

"contributes": {
  "notebookOutputRenderer": [
    {
      "viewType": "sample-notebook-renderer",
      "displayName": "Sample Notebook Renderer",
+     "entrypoint": "./out/rendererUiCode.js"
      "mimeTypes": [
        "application/json"
      ]
    }
  ],

I find this approach attractive as it is much easier to implement outside of VS Code, and also much easier to consume and build with -- there's no pseudo-Node/require based environment, no multiple targets like the renderer starter has, no figuring out how to load your files. You just have a bundle of JavaScript code to be loaded in the UI.

The downside is that this closes the door for what Jackson called "static renderers", and if there is anything that renderers need to do outside the DOM, they cannot do that. Arguably, though, DOM restrictions is a benefit for portability, and making every renderer 'dynamic' reduces the number of concepts/moving parts the user needs to learn.

In-Webview API

The current VS Code notebook renderer API is defined here: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/vscode-notebook-renderer/index.d.ts

This is already a fairly minimal API that neither path will require to change significantly, however moving Events to a separate package would ease implementations.

In Path 2, there's no Node side of things to receive the per-renderer postMessage events, but the content provider may still receive them.

It would make sense to add an additional product field in the event that renderers need to distinguish between different products. For example, we support command links from renderer output in VS Code, but those won't work (at least, not with perfect parity) in ADS.

---

cc @rebornix @roblourens @DonJayamanne

[roblourens]

The downside is that this closes the door for what Jackson called "static renderers", and if there is anything that renderers need to do outside the DOM, they cannot do that

I don't understand this, what's missing? Is it the fact that the renderer code wouldn't be loaded until it's needed or something?

[roblourens]

How much will the renderer typings grow? It doesn't really seem worth the trouble to share types between vscode.d.ts and the renderer dts just for Event and Disposable.

Also do extensions in serverless get a subset of vscode API?

[connor4312]

I don't understand this, what's missing? Is it the fact that the renderer code wouldn't be loaded until it's needed or something?

Currently, you can render 'static' HTML in the extension host and not have any JavaScript running in the client side. This proposal would make it all client-side, although the actual construction logic could be very similar to a static renderer if the author desires.

E.g. you can do whatever building you want in the return value from

vscode/src/vs/vscode.proposed.d.ts

Lines 1640 to 1645 in 48f23d8

	/**
	*
	* @returns HTML fragment. We can probably return `CellOutput` instead of string ?
	*
	*/
	render(document: NotebookDocument, request: NotebookRenderRequest): string;

But in the starter (and yeoman template) I render a script tag which is picked up by my client-side code.

https://github.com/microsoft/vscode-notebook-renderer-starter/blob/304063a42245f0635c0f71b551a94d0091b0ee7b/src/extension/sampleRenderer.ts#L36-L40

How much will the renderer typings grow? It doesn't really seem worth the trouble to share types between vscode.d.ts and the renderer dts just for Event and Disposable.

The renderer APIs also currently include the NotebookDocument, which brings in a bunch stuff like RelativePattern, which brings in WorkspaceFolder, etc. Have that stuff in two places seems error-prone.

[connor4312]

Notes from sync where I pitched Path 2 as my preferred solution:

• Should we have them acquire the global API or export an activate function like we do for extensions?
• Not everyone would want to write client-side code
  • Joh mentioned in the Github issues notebook that he does server-side rendering and it's painful
  • Build setup would be taken care of in yeoman generator
  • We should investigate renderers made thus far and see what people are doing
• Potential performance issues doing transformation of data in the renderer
  • e.g. having content providers that generate lots of data that can be slow to calculate, esp on platforms where iframes are not separate processes (e.g. iPad)
  • we could have a way to have a transformation function be done in a worker
  • the extension author could also make parts of their code worker-based on their own
• Potential performance issues sending 'raw' data to the webview (e.g. hexeditor scenario)
  • However it's already often the case that the renderer extension (UI) will not live in the sample place as the content provider (workspace)
• Peng mentioned the concept of data transformers for notebooks
  • Transform from one mime type to another
  • However this would generally be useful for when you don't own the renderer -- otherwise you can just have the renderer deal with it directly
  • Doesn't necessarily solve problems, since a transformer could equally reduce or increase the amount of data sent to the output

Followup for me: looking at the existing renderers to see what developers have done and whether they could be supported in the Path 2 scenario

[connor4312]

Assessment of existing renderers:

• Github issues notebook (adopt pure renderer/client-side ui vscode-github-issue-notebooks#62)
  • Just renders static HTML and outputs the HTML mime type
  • To convert it to a client-only renderer, I had to:
    • Update the build. Already used webpack. I tried to just have multiple entrypoints but webpack only allows setting the libraryTarget on a per-compilation basis, so need to run webpack twice
    • Move things over from the HTML renderer. Easy enough. There's no usage of webview APIs at the moment.
    • To be correct (e.g. not have jsx enabled for the extension side of things or Node types in the renderer side) and also share common interfaces, I needed to create a three-package structure like the starter had
  • However, we can probably just have a built-in mime type renderer for HTML to avoid requiring changes here.
  • Observation: if we don't have an HTML mime type renderer out of the box, then content providers are encouraged to generate structured data and generate HTML in a bespoke renderer. This will make it easier to write Transformers and other renderers as you're less likely to need to HTML-scrape the data you need from a content provider. Joh's nice in that he provides both HTML and structured data, but notebook developers may not do this. This is more build setup, but it can be provided in a yeoman starter.
  • The regexper sample is the same archetype
• Markdown notebook
  • Just outputs markdown cells. No changes here.
• Kusto Notebook
  • This converged with or copied the pattern I originally used in renderer starter (before the acquireNotebookRendererApi was available) where the renderer in the extension host just stringifies output data to be sent to the nteract client-side code (ref)
  • It does not using the acquireVSCode API
  • Moving this to client-side is straightforward
  • The nteract sample is the same archetype
• Don's Plotly/Vega
  • Uses the starter yeoman code, with modifications:
    • Sends the complete output cell rather than only the mime type in question
    • Manually calls render functions (like the starter did pre-acquireNotebookRendererApi)
  • @DonJayamanne I'm curious why these changes were needed :)
  • Otherwise, pure client-side renderer

With the renderers we have it seems that Path 2 is feasible, however ipywidgets is the not-yet-implemented weird case.

Some notes from the sync today:

• ipywidgets remain the 'weird' case where the kernel needs client code and communication with the renderer
  • I do not want to design a large/specialized API for a case where there's only one known consumer
  • I think the kernel preload script strategy we have today is probably 'good enough'. As mentioned above, with renderers being client-side only there's no longer any purpose to namespacing, so postMessage on the renderer API will be sent to all kernels.
  • We will probably need to annotate where the postMessages come from so the kernel can distinguish between multiple open notebooks or editors
• Agreement that transformers are out of scope for the renderer discussion

[DonJayamanne]

I'm curious why these changes were needed :)

This is an old implemetation. I haven't updated that to use the new API.
Here' the temporary renderer in a different repo https://github.com/microsoft/vscode-python/blob/master/src/datascience-ui/renderers/index.tsx
It will be moved into the renderer extension once we have sorted out a few other issues.

[rebornix]

Some undocumented rules/conversion in the renderer/kernel preload scripts API:

• Kernel preload scripts are loaded when the kernel is activated/connected
• Renderer preload scripts are loaded only when the renderer is picked for a specific mimetype (automatically or manually)
• Preload scripts are resource URIs and one URI will only be inserted into the DOM node once
• Preload scripts are inserted into the DOM int the same sequence as in preloads: Uri[]
• A preload script is inserted into the DOM as <script src="...">

cc @DonJayamanne

[connor4312]

This is now implemented, and the impure API is removed.