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

Support Notebook Renderers with VS Code Notebooks (disabled) #11999

Merged
merged 19 commits into from
May 28, 2020

Conversation

DonJayamanne
Copy link

@DonJayamanne DonJayamanne commented May 26, 2020

For #10496

  • Add notebook renderers
  • Please note there's a big hack in this code (its to get around a VS Code issue).
  • Apart from that the rest should be ok

Unfortunately some stuff isn't working properly, however I'd like to get this reviewed. The fixes should be simple.

.vscode/launch.json Outdated Show resolved Hide resolved
@DonJayamanne DonJayamanne marked this pull request as ready for review May 26, 2020 23:34
Copy link

@rchiodo rchiodo left a comment

Choose a reason for hiding this comment

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

:shipit:

@@ -344,7 +356,8 @@
"XCI_PYTHON_PATH": "<Python Path>"
},
"outFiles": [
"${workspaceFolder}/out/**/*.js"
"${workspaceFolder}/out/**/*.js",
"!${workspaceFolder}/**/node_modules**/*"
Copy link
Author

Choose a reason for hiding this comment

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

New changes due to new debugger in VS Code (we'll need this to ensure we ignore node_modules in the out folder). Else debugger is very slow to load, causing extension to take 5-8 minutes to activate.

interactiveWindow: ['babel-polyfill', `./src/datascience-ui/history-react/index.tsx`]
};
function getEntry(bundle) {
switch (bundle) {
Copy link
Author

Choose a reason for hiding this comment

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

Switched to named bundles instead of a boolean.

@@ -298,7 +320,13 @@ function buildConfiguration(isNotebook) {
]
}
};

if (bundle === 'renderers') {
delete config.optimization;
Copy link
Author

Choose a reason for hiding this comment

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

No optimization for renderer scripts, they don't work at all.
Will be resolved when VS Code adds support for loading split bundles.

DonJayamanne added a commit that referenced this pull request May 28, 2020
For #11999

When running unit tests we don't need to compile the webviews (no need to compile react, etc).
Total time to compile code down from 6 minutes to 1 minute (i.e. 5 minutes faster CI to know whether tests passed/failed)
Note the drop in compile times from ~7minutes to ~50s. (basically 6 minutes faster)
@sonarcloud
Copy link

sonarcloud bot commented May 28, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@DonJayamanne DonJayamanne changed the title Notebook Renderers Support Notebook Renderers with VS Code Notebooks (disabled) May 28, 2020
@DonJayamanne DonJayamanne merged commit cc93436 into microsoft:master May 28, 2020
@DonJayamanne DonJayamanne deleted the renderers branch May 28, 2020 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog No news entry required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants