Skip to content
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

Extensible markdown notebook renderer api #121256

Closed
mjbvz opened this issue Apr 14, 2021 · 9 comments · Fixed by #151467
Closed

Extensible markdown notebook renderer api #121256

mjbvz opened this issue Apr 14, 2021 · 9 comments · Fixed by #151467
Assignees
Labels
api api-finalization feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders notebook-markdown on-testplan
Milestone

Comments

@mjbvz
Copy link
Collaborator

mjbvz commented Apr 14, 2021

Tracks adding an official API for extending markdown rendering in notebooks. Internally we've already implemented and shipped basic support for this, but this api is not suitable for public consumption

This is a follow up on #106701/#116393

Background

There are a range of different markdown flavors used across the various notebook ecosystems. Some notebooks support inline math formulas for example, while others use specific markdown parsers with their own set of extensions/quirks

VS Code can not include all of the potential markdown features that a given notebook may want, so we instead plan to allow markdown rendering to be extended (and potentially replaced) by extensions. There are two main use cases we are interested in:

  • Augmentation: An extension wants to augment an existing markdown parsers with a simple new feature, such as :emoji: syntax support

  • Replacement: An notebook requires a fully custom renderer for markdown

Current state

With #106701, we added the ability to fully replace VS Code's built-in markdown rendering with a renderer provided by an extension. The new markdown rendering also happens entirely in the back layer webview of notebook (similar to output renderers). At the moment, only built-in extensions can replace VS Code's markdown renderer

We are using this api to render notebook markdown with markdown-it provided by a built-in extension.

We also added an adhoc api that lets the markdown-it renderer load markdown-it plugins. We are using this to load katex and markdown-it-emoji support

Neither of these apis are ready for external consumption

Some open questions

  • Should we make this generic?

    For example, instead of restricting this to markdown, are there notebooks out there that use plaintext or asciidoc for their inputs?

    Should we have an "input renderer" API that matches how our "output renderer" api works

  • How do these features fit with kernels and the rest of notebook apis?

    Should a kernel be able to specify which flavor of markdown it requires?

    Does it make sense to let a separate extension add support for new markdown syntax like :emoji:, or should this always come from the notebook provider itself?

  • Does fully replacing our markdown rendering even make sense for anyone besides markdown it?

    When a notebook ecosystem does use a specific markdown flavor, it is usually powered some non-js tool. This means we cannot load it in the webview, so at a minimum we'd have to provide some way to let the renderer call back to extension code so that it can run a native script. This sounds like a recipe for lots of delays with the rendering

    I guess the main question is if markdown-it is flexible enough so that it could handle most of the (common) notebook use cases. If it is, we will not need to support fully replacing the markdown renderer

@mjbvz mjbvz added this to the On Deck milestone Apr 14, 2021
@mjbvz mjbvz self-assigned this Apr 14, 2021
@mjbvz
Copy link
Collaborator Author

mjbvz commented Apr 14, 2021

Prototype

Here's an approach I've prototyped for the api. This tries to support both replacement and augmentation of renderers, although we may ultimately decide that we only want to support extending a version of markdown-it that we ship (similar to what our built-in markdown extension supports)

Contribution point

For a renderer replacement:

"contributes": {
  "notebookMarkdownRenderer": [
    {
      "id": "markdownItRenderer", // Unique id of renderer
      "displayName": "Markdown it renderer", // Name shown to user in UI 
      "entrypoint": "./notebook-out/index.js" // JS file loaded into the backlayer webview
    }
  ]
}

For a renderer that augments another one:

"contributes": {
  "notebookMarkdownRenderer": [
    {
      "id": "markdownItRenderer-emoji",
      "displayName": "Markdown it emoji renderer",
      "entrypoint": "./notebook-out/emoji.js",

      // The optional 'dependsOn' lets us know that this renderer should only be loaded if
      // 'markdownItRenderer' is being used
      "dependsOn": "markdownItRenderer" 
    }
  ]
}

Entrypoints

First for an augmentation, in this case emoji:

// The entry point is a module with a `extendMarkdownIt` export

import type * as markdownIt from 'markdown-it';

const emoji = require('markdown-it-emoji');

// This function is involved by the markdown-it renderer once, before any markdown is rendered
//
// Here we are passed the markdown-it instance to augment 
//
// This matches our markdown extension api: https://code.visualstudio.com/api/extension-guides/markdown-extension#adding-support-for-new-syntax-with-markdownit-plugins
export function extendMarkdownIt(md: markdownIt.MarkdownIt) {
    md.use(emoji);
}

Now for the markdown-it renderer itself, which fully replaces VS Code's markdown renderer:

// Again the entry point is a JS module

const MarkdownIt = require('markdown-it');

// Here we declare that the global notebook api exists
declare const acquireNotebookRendererApi: any;

// Our entry point module exports am `activate` method which is given 
// a set of renderers that depend on it. This list is generated from the
// `depends-on` contribution property.
// 
// In this example, `activate` would be passed the `emoji` module defined above
export async function activate(ctx: {
    dependencies: ReadonlyArray<{ entryPoint: any }>
}) {
    const markdownIt = new MarkdownIt({
        html: true
    });
    
    // Here we load all the extensions to markdown it
    for (const dep of ctx.dependencies) {
        try {
            dep.entryPoint?.extendsMarkdownIt?.(markdownIt);
        } catch (e) {
            console.error('Could not load markdown entryPoint', e);
        }
    }

    const notebook = acquireNotebookRendererApi('notebookCoreTestRenderer');

    // This is where we do the actual replacement of the markdown renderer currently 
    //
    // Note: it would probably be better if this did not rely on a magical global event like this.
    // Could `activate` return the render method instead?
    notebook.onDidCreateMarkdown((markdownCtx: { element: HTMLElement, content: string }) => {
        // Render the markdown

        // Do we also need a way to the markdown extensions to be invoked here?
        // Katex for example needs to add a style sheet for math formula.
        // We render the markdown in a shadowdom, so this stylesheet must be copied
        // into `markdownCtx.element`

        const rendered = markdownIt.render(markdownCtx.content);
        markdownCtx.element.innerHTML = rendered;
    });

}

Notable Problems

  • The above may be overly complicated. Specifically if we only need to support augmenting markdown-it, we don't have to standardize all the weird globals stuff going on in the entrypoint above

  • Still, it might be nice to define a proper api for replacing markdown renderers, even if we just use it internally. notebook.onDidCreateMarkdown is pretty ugly

  • It would be really nice if we could defer loading resources until we really need them (or at least not block notebook open because we are loading resources that are never used)

    For example, if a notebook supports math but none of its cells use math, we should not load katex. We can quickly check if any of the cells may contain math equations just by seeing if they contain at least two dollar sign characters somewhere in the text

@jrieken
Copy link
Member

jrieken commented Apr 16, 2021

Should we make this generic?

This is an interesting thought. We have discussed if we should blur the line between code and markdown cells, e.g there is just cells with output and cells using the markdown language get a different cell editor

mjbvz added a commit that referenced this issue Apr 22, 2021
For #121256

- Use js modules for notebook output renderers
- Rename apis from `markdown` to `markup`
- Use imports and exports for apis instead of globals for apis
- Use esbuild instead of webpack so we can emit js modules
- Clearly split top level markup renderes from renderers that extend other renderers
@mjbvz
Copy link
Collaborator Author

mjbvz commented Apr 22, 2021

#121882 Implements the proposed API. Performance seems about the same

I also renamed the extension point from markdownRenderer to markupRenderers to prototype what a more generic api would look like

@jrieken
Copy link
Member

jrieken commented Apr 22, 2021

This will be very similar to "normal" output renderers. I like that direction and wonder if they can be the same at one point?

mjbvz added a commit that referenced this issue Apr 26, 2021
For #121256

- Use js modules for notebook output renderers
- Rename apis from `markdown` to `markup`
- Use imports and exports for apis instead of globals for apis
- Use esbuild instead of webpack so we can emit js modules
- Clearly split top level markup renderes from renderers that extend other renderers
mjbvz added a commit that referenced this issue Apr 26, 2021
* Better notebook markup renderer api

For #121256

- Use js modules for notebook output renderers
- Rename apis from `markdown` to `markup`
- Use imports and exports for apis instead of globals for apis
- Use esbuild instead of webpack so we can emit js modules
- Clearly split top level markup renderes from renderers that extend other renderers

* Use constant instead of comment for replacement
@choldgraf
Copy link

Just a quick note that we would find this API very useful, thanks for working on it :-)

We work with the Executable Books Project which builds the tool Jupyter Book. We're also maintaining a flavor of markdown called MyST Markdown, which has extensible syntax and features more suited for scientific communication.

We have a MyST Markdown renderer for VSCode, and plan to incorporate more MyST Markdown into Jupyter Notebooks. This API would let us do so in VSCode relatively easily via the markdown renderer above, which would be super cool 🎉

@mjbvz
Copy link
Collaborator Author

mjbvz commented Oct 6, 2021

Before finalizing this, we should also consider preloads for renderers: #132557

I think these can probably slot into the notebookRenderers contribution

@mjbvz mjbvz added the feature-request Request for new features or functionality label Oct 11, 2021
@mjbvz mjbvz modified the milestones: October 2021, November 2021 Oct 12, 2021
@mjbvz mjbvz modified the milestones: November 2021, On Deck Nov 2, 2021
@DonJayamanne
Copy link
Contributor

DonJayamanne commented Mar 7, 2022

As requested @mjbvz
Please could we provide an API whereby the renderer can access Notebook Cell metadata when rendering Notebook markdown cells.

Usage in Jupyter extension:

mjbvz added a commit to mjbvz/vscode that referenced this issue May 18, 2022
For microsoft#121256

This change adds the current `ouputItem` to the notebook markdown renderer's environment

Renders that extend our markdown renderer can use this to access output item metadata for example
@mjbvz
Copy link
Collaborator Author

mjbvz commented May 18, 2022

@DonJayamanne #149870 Lets a renderer that extends our built-in markdown rendering access the current cell metadata (you would do this while rendering by using markdown-it's env property)

Let me know if this is enough for your use cases. I'm actually not sure if images are stored in the cell's metadata or in the notebook's metadata (if it's the latter, we would also need to implement #149869)

@mjbvz mjbvz modified the milestones: On Deck, June 2022 May 18, 2022
mjbvz added a commit that referenced this issue May 18, 2022
For #121256

This change adds the current `ouputItem` to the notebook markdown renderer's environment

Renders that extend our markdown renderer can use this to access output item metadata for example
@DonJayamanne
Copy link
Contributor

Will try this out and let you know, thanks

mjbvz added a commit to mjbvz/vscode that referenced this issue Jun 7, 2022
@vscodenpa vscodenpa added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Jun 8, 2022
mjbvz added a commit to mjbvz/vscode that referenced this issue Jun 14, 2022
For microsoft#121256

Use `allOf` to allow two versions of a renderer contribition: one for direct renderers and one for renderers that extend other renderers
mjbvz added a commit that referenced this issue Jun 14, 2022
For #121256

Use `allOf` to allow two versions of a renderer contribition: one for direct renderers and one for renderers that extend other renderers
@github-actions github-actions bot locked and limited conversation to collaborators Jul 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api api-finalization feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders notebook-markdown on-testplan
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants