Standardizing notebook renderer api

[mjbvz]

Tracks standardizing (and potentially unifying) the notebook markup and output renderer apis. These two should at least be similar shapes so that they are easier to work with

Split from #105933

Current state

We recently added extensible markdown renderers to notebooks. This means that there are now two types of renderer inside a notebook's backing webview:

• Output renderers — These render cell outputs
• Markup renderers — These render markdown cells. Currently we only allow them to be contributed by extensions

Output renderers are implemented using scripts rely on a global acquireNotebookRendererApi function:

const notebook = acquireNotebookRendererApi();

interface ICellCreateInfo {
    readonly element: HTMLElement;
   
    readonly mime: string;
    readonly value: unknown;
    readonly metadata: unknown;
}

// Renderer can do some static setup here


// Then registers the actual renderer function with VS Code
notebook.onDidCreateOutput(({ element, mimeType }: ICellCreateInfo) => {
	const div = document.createElement('div');
	div.innerText = `Hello ${mimeType}!`;
	element.appendChild(div);
});

Markup renderers originally took the same approach, but we later switched them to use JS modules for a few reasons:

• It make it easier to express renderers that depend on other renderers without using globals. For example, our builtin markdown-it renderer is extended by another markup renderer that adds KaTeX support
• Modules are nicer to work with for modern JS
• It let us better define how the renderer api is used. For example, it lets our webview code call out into the renderer when it needs to instead of letting renderers register themselves with notebook.onDidCreateOutput

To power these two types of renderers, we also have two types of contribution points in extensions: notebookOutputRenderers and notebookMarkupRenders

Goal

After working with and having to maintain these two separate renderer APIs, we believe it makes sense to unify them. A unified renderer API would let extension authors focus on rendering without having to worry about where the rendered output is used.

We also think that the existing markup renderer api is nicer to work with and more future proof, so we'd like to move the existing APIs

Here's a summary of what we are trying to achieve:

• Give extension authors a modern JS environment and familiar code structure to build their renderers.
• Define APIs that can be easily extended in the future
• Make it easy for extensions to defer loading large resources until they are needed.
• Make it simple to define renderers that can be extended by other renderers. This is especially important for markdown.
• Try to make these changes without breaking existing renderers until they have had a chance to upgrade themselves.

Proposed design

Here's the current proposal for a unified renderer API. This would be the code implemented by an extension and loaded in the backlayer notebook webview. Note that while it looks quite long compared to the existing API, this is mainly because I've called out all the types involved. The actual code an extension would need to implement should only be a few lines longer than what you have to write today

// In renderer

// Information about a cell to be rendered
//
// Renderer extensions would consume this interface through a `d.ts` file. It is included here for clarify
interface ICellInfo {
    readonly kind: 'markup' | 'output';

    readonly element: HTMLElement;
   
    readonly mime: string;
    readonly value: unknown;
    readonly metadata: unknown;

    // Potentially expose other fields here, such as URIs
}


// The main entrypoint of a renderer would be this `activate` function
// VS Code would call this when the renderer is need. This is inspired by the VS Code
// extension API.
//
// `activate` lets VS Code pass data to the renderer. It is also where we return the rendering
// functions that VS Code  will invoke later.
export function activate(
    context: RendererContext
): RendererApi {
    
    // Renderer can do some static setup here

    // Then we return the actual renderer implementation
    return {
        renderCell: (id: string, cell: ICellInfo) => {
            const div = document.createElement('div');
            div.innerText = `Hello ${cell.mimeType}!`;
            cell.element.appendChild(div);
        },
    };
}

// This object is passed to the renderer from VS Code
interface RendererContext {

     // get the static specific to the renderer.
     // This would have the same functionality as acquireNotebookRendererApi().getState()
     getState<T>(): T;

     // Get the static specific to the renderer.
     // This would have the same functionality as acquireNotebookRendererApi().setState()
     setState<T>(newState: T): void;

    // Access another renderer's exports (the object returned from `activate`)
    // This is needed to allow a markdown extension to extend another one
    getRenderer<T>(id: string): undefined | T;
}

// This object defines what the renderer returns to VS Code
interface RendererApi {

    // Render a given cell.
    // @returns an optional dispose method called when the cell element is destoryed
    renderCell(id: string, cell: ICellInfo): undefined | { dispose: () => void };
}

Example implementation

#123738 adopts an early cut of this API for rendering markdown cells. The main files to look at:

• markdownIt-renderer — This is the top level markdown renderer

• markdownIt-emoji-Renderer — This shows how one renderer can extend another one

Open questions

• Should we try aligning kernel preloads with this API too?

  May get complicated if you need to define globals

• Should any parts of the api allow returning promises instead? renderCell for example?

• Should we block extensions from replacing our markdown renderer for markup cells?

  For now, probably yes because tracking down why markdown rendering is weird would be difficult. Good to support in the future though

[connor4312]

This looks sensible to me. @vijayupadya is the point of contact for aznb.

For this we would switch to loading to loading renderers via ES6 import rather than <script> tags? I like that, script tag loading is ugly.

Should we apply the this treatment to kernel preloads? This could make them slightly harder to author -- for example, I could no longer just take jquery.min.js and stick it in a preload -- but would standardize all renderer script loading and remove a lot of code from our webviewPreloads.ts.

Wrt switchover: we can probably support both safely for a bit; if the import() fails (which will happen if there's no export, I believe) then fall back to the old code path.

Adding myself on here to help out 🙂

[DonJayamanne]

Would it be possible to provide some information that would help extension authors link a cell in a notebook to an output that's being rendererd?

interface ICellCreateInfo {
	cellUri: Uri
    readonly kind: 'markup' | 'output';

longer just take jquery.min.js and stick it in a preload -- but would standardize all renderer script loading and remove a lot of code

Is it possible to have a dependency or preloads section. In Jupyter we'd like to load requir.js (though this can be done via bundling as well), but it would prevent us from having to bundle require.js in the kernel preloads as well as the renderer extension.

[mjbvz]

@connor4312 Yes we'd want to use dynamic imports instead. I have this implemented with the existing markup renderers

Not sure about kernel preloads since I'm not as familiar with them. Is the concern that you'd no longer be able to pull in a jquery global to make it accessible in renderer scripts? Could preload assign to window. if it wants to export a global variable?

[connor4312]

I remember TypeScript producing errors trying to import a file without exports, but it looks like in browsers I can do something like import('data:application/javascript,window.foo="bar"') and it works fine. The downside is that, in the module context, this is undefined. Many pre-bundled JavaScript code depends on this being the window in the global scope.

When do you plan to implement this? I would like to get #123601 done tomorrow (if uncontroversial) to unblock Don, but if you think this is ready to go then those IPC changes can be done afterwards.

Looking forward to this refactor, it'll make lots of things nicer 👍 Let me know if there's any way I can assist or parts you would like me to implement.

[mjbvz]

@connor4312 Here's a first cut at implementing the proposed API: #123738. I could use your help properly hooking up the output renderers and making sure this won't cause regressions for them

Can you also please make sure the AZ notebooks team is aware of this work. I've added significant background in the original comment which should better explain the rational and benefits of this proposed change

[DonJayamanne]

When rendering markdowns, we'll need to know the cell uri, so we can pull some metadata from the cell notebook. See here #123021

[jrieken]

When rendering markdowns, we'll need to know the cell uri, so we can pull some metadata from the cell notebook. See here #123021

Should we consider to send the cell data (or the cell metadata) as part of the renderer request? This wouldn't be mutual exclusive to renderer IPC but certainly an easy solution for this

[connor4312]

Yes, that should be part of ICellInfo. Without renderer IPC it's not useful, but it's cheap to have.