microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
162.93k stars 28.77k forks source link

Enable localization of notebook renderers #170919

Open mjbvz opened 1 year ago

mjbvz commented 1 year ago

Tracks enabling localization for notebook renderer scripts. This work similarly to our existing extension localization support

mjbvz commented 1 year ago

My proposal is to expose the l10n API in the context parameter passed to the notebook renderer:

// Only showing new fields
export function activate(ctx: {
    l10n: {
        language: string, // For example, en-us

       t(...) => string, // Localize function. Matches what we have in VSCode api

       bundle: { ... } // Complete string bundle. Matches what we have in VSCode api
    }
})

@TylerLeonhardt was also interested in seeing if we could make l10n work more like it does for VS Code extensions. Passing around the context everywhere could be annoying, so I considered two approaches to this. However I am not sure either one can be implemented:

mjbvz commented 1 year ago

Look into import maps: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script/type/importmap

https://github.com/microsoft/vscode/blob/b42832fd4d1dc80d7095b5f0f4bdde3fe598e5cd/test/unit/electron/renderer-esm.html#L51

mjbvz commented 1 year ago

New proposal using import maps:

In notebook renderer:

import * as vscode from `vscode-notebook-renderer`

...
vscode.l10n.t("My string")

The shape of l10n here would match l10n from vscode.d.ts


Unfortunately we likely can't ship this until import maps are also supported in mainline safari

mjbvz commented 1 year ago

Actually we may be able to ship, but extensions would need to handle unsupported browsers themselves. Basically they would do:

let vscode;
try {
    vscode = await import('vscode-notebook-renderer')
} catch {
    vscode = { l10n: { t(msg) { return msg } }}
}

Since I expect few extensions to adopt l10n immediately and since Safari technical preview already supports import maps, this is likely an acceptable workaround until support lands in mainstream safari

TylerLeonhardt commented 1 year ago

@mjbvz this changes the API from sync to async. Do we have top level await support?

mjbvz commented 1 year ago

@TylerLeonhardt The api will still be sync, but the import is async. The should work using a top level await in the notebook renderer module

mjbvz commented 1 year ago

Safari 16.4 added import maps support so we should be unblocked for all modern browsers. Tentatively assigning to June