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

Enable localization of notebook renderers #170919

Open
mjbvz opened this issue Jan 10, 2023 · 7 comments
Open

Enable localization of notebook renderers #170919

mjbvz opened this issue Jan 10, 2023 · 7 comments
Assignees
Labels
api-proposal feature-request Request for new features or functionality l10n-platform Localization platform issues (not wrong translations) notebook
Milestone

Comments

@mjbvz
Copy link
Collaborator

mjbvz commented Jan 10, 2023

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

@mjbvz
Copy link
Collaborator Author

mjbvz commented Jan 10, 2023

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:

  • Fake imports: import { l10n } from 'vscode';

    On web, I don't think it's possible to express this. Imports need to resolve to a resource that actually exists

  • Globals: vscode.l10n.t(...)

    Because notebook renderers run as modules, this is not easy to do. We'd need a "global" variable that is scoped just to the current module. Either that or the global api has to take the notebook render id as a argument so that it knows which extension's strings it should return

@TylerLeonhardt TylerLeonhardt added l10n-platform Localization platform issues (not wrong translations) feature-request Request for new features or functionality labels Jan 10, 2023
@mjbvz
Copy link
Collaborator Author

mjbvz commented Jan 17, 2023

mjbvz added a commit that referenced this issue Jan 17, 2023
For #170919

This shows how we can use importmap to create an api that is specific to each renderer. This lets a renderer write:

```ts
import * as vscode from 'vscode'
```

and get back a version of that api that is specific to that renderer, even though multiple different renderers may be running in the same notebook webview
@mjbvz
Copy link
Collaborator Author

mjbvz commented Jan 17, 2023

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
Copy link
Collaborator Author

mjbvz commented Jan 19, 2023

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
Copy link
Member

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

@mjbvz
Copy link
Collaborator Author

mjbvz commented Jan 19, 2023

@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
Copy link
Collaborator Author

mjbvz commented May 25, 2023

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

@mjbvz mjbvz modified the milestones: June 2023, July 2023 Jun 12, 2023
@mjbvz mjbvz modified the milestones: July 2023, August 2023 Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-proposal feature-request Request for new features or functionality l10n-platform Localization platform issues (not wrong translations) notebook
Projects
None yet
Development

No branches or pull requests

2 participants