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

JavaScript evaluated by the Notebook renderer is evaluated in the global scope #157076

Closed
DonJayamanne opened this issue Aug 4, 2022 · 2 comments · Fixed by #160347
Closed

JavaScript evaluated by the Notebook renderer is evaluated in the global scope #157076

DonJayamanne opened this issue Aug 4, 2022 · 2 comments · Fixed by #160347
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders notebook verified Verification succeeded

Comments

@DonJayamanne
Copy link
Contributor

DonJayamanne commented Aug 4, 2022

Does this issue occur when all extensions are disabled?: Yes/No

  • VS Code Version: All
  • OS Version: All

Steps to Reproduce:

  1. Create a jupyter notebook with Pyhton
    2.Run the following cell twice and have a look at the console output in the renderrers
%%javascript
const helloWorld = 2134;

Basically the error indicates the fact that the const helloWorld has already been declared once before.

The problem is we're evaluating the JS in the same global scope.
Jupyter doesn't seem to be evaluating them in the global scope.

Here's the problematic code

function renderJavascript(outputInfo, container) {
  const str = outputInfo.text();
  const scriptVal = `<script type="application/javascript">${str}</script>`;
  const element = document.createElement("div");
  const trustedHtml = ttPolicy?.createHTML(scriptVal) ?? scriptVal;
  element.innerHTML = trustedHtml;
  container.appendChild(element);
  domEval(element);
}

Not sure if others (extensions) rely on this to run in the global scope, hence I'd suggest adding a metadata for JS mimetypes to ensure the code is evaluated in a private scope, e.g. isolated.
Jupyter has similar concepts when rendering HTML, https://ipython.org/ipython-doc/3/notebook/nbformat.html#output-metadata

Perhaps we could do the same thing, to ensure the existing functionality stays as is (for the benefit of existing extension authors).

@DonJayamanne
Copy link
Contributor Author

@mjbvz I'd like to work on addressing this issue, we can sync up next week or some time tomorrow.

@mjbvz mjbvz added the notebook label Aug 5, 2022
@DonJayamanne DonJayamanne self-assigned this Aug 22, 2022
@mjbvz mjbvz added the bug Issue identified by VS Code Team member as probable bug label Sep 7, 2022
@mjbvz mjbvz added this to the September 2022 milestone Sep 7, 2022
mjbvz added a commit to mjbvz/vscode that referenced this issue Sep 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 Sep 7, 2022
@mjbvz mjbvz added the author-verification-requested Issues potentially verifiable by issue author label Sep 29, 2022
@vscodenpa
Copy link

This bug has been fixed in the latest release of VS Code Insiders!

@DonJayamanne, you can help us out by commenting /verified if things are now working as expected.

If things still don't seem right, please ensure you're on version 2c80125 of Insiders (today's or later - you can use Help: About in the command palette to check), and leave a comment letting us know what isn't working as expected.

Happy Coding!

@rzhao271 rzhao271 added verified Verification succeeded and removed author-verification-requested Issues potentially verifiable by issue author labels Oct 3, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Oct 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders notebook verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants