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

Add static preloads notebook contribution #163512

Merged
merged 1 commit into from
Oct 14, 2022
Merged

Conversation

mjbvz
Copy link
Collaborator

@mjbvz mjbvz commented Oct 13, 2022

For #163511

@DonJayamanne Right now I'm forcing all of these static preloads to be loaded as modules that export an activate function. You can then use this to load additional scripts into the webview

If we need to, we can instead explore loading these preloads as non-module scripts. However these would only be evaluated in the global scope. I do not want to support the old school kernel preload API where we injected global variables with context info

@mjbvz mjbvz self-assigned this Oct 13, 2022
@DonJayamanne
Copy link
Contributor

Thanks for this change, I will have a look at this today and get back to you. Should be fine the way it is.
I dont see this changing any existing renderers or kernel scripts.


export interface INotebookPreloadContribution {
readonly [NotebookPreloadContribution.type]: string;
readonly [NotebookPreloadContribution.entrypoint]: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will these scripts have access to the messaging channel, i.e. can the webview scripts send messages back to the exension host?
From what I can tell they can, however I'm not sure how the extension can listen to these messages, as we don't have an identifier to setup a comms channel.

Note: This was never a requirement, just curious about the capabilities.

@DonJayamanne
Copy link
Contributor

Thanks for this change, I will have a look at this today and get back to you. Should be fine the way it is.
I dont see this changing any existing renderers or kernel scripts.

@DonJayamanne
Copy link
Contributor

as modules that export an activate function.

Is the requirement to just expose an activate function without any arguments, or are there some arguments passed into the active function?

@DonJayamanne
Copy link
Contributor

Thanks @mjbvz
I created a simple js file with an actiavte and include the contents of require.js into that, and this works as expected.
I'll whip up the PRs in Jupyter extension next week (my Monday, your sunday) to make use of this API,
with the plan of removing vscodes AMD loader from being exposed in the webviews.

@mjbvz
Copy link
Collaborator Author

mjbvz commented Oct 14, 2022

Thanks for testing @DonJayamanne!

I'll merge this initial version and we can keep iterating on it next week

@mjbvz mjbvz merged commit 6451298 into microsoft:main Oct 14, 2022
@mjbvz mjbvz added this to the October 2022 milestone Oct 14, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Nov 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants