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

Unsaved notebook fails to render when a folder is added to the workspace #173219

Closed
minsa110 opened this issue Feb 2, 2023 · 4 comments
Closed
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug notebook

Comments

@minsa110
Copy link

minsa110 commented Feb 2, 2023

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

Version: 1.75.0-insider (user setup)
Commit: e2816fe
Date: 2023-02-01T15:16:18.248Z
Electron: 19.1.9
Chromium: 102.0.5005.194
Node.js: 16.14.2
V8: 10.2.154.23-electron.0
OS: Windows_NT x64 10.0.22621
Sandboxed: Yes

Steps to Reproduce:

  1. Open a folder
  2. Make changes to an ipynb file but don't save
  3. Add a folder to the workspace
  4. Click "Cancel" when prompted to save
  5. The ipynb file doesn't render in notebook view (I could open it as json if I click "Open in Text Editor" though)
  6. Workaround is to reload the window

new workspace ipynb rendering issue

@bpasero
Copy link
Member

bpasero commented Feb 3, 2023

The underlying reason is very likely the fact that we restart the extension host which is tough for any editor that requires the EH to work (for example custom editors).

Here the stack trace is:

notebookEditor.js:194 TypeError: Cannot read properties of undefined (reading 'onDidChangeContent')
    at NotebookEditor.setInput (notebookEditor.js:154:64)
    at async EditorPanes.doSetInput (editorPanes.js:292:17)
    at async EditorPanes.doOpenEditor (editorPanes.js:166:44)
    at async EditorPanes.openEditor (editorPanes.js:65:24)
    at async editorGroupView.js:809:65

I suspect some disposal code running from notebooks clearing out the model.

@bpasero bpasero assigned roblourens and rebornix and unassigned bpasero Feb 3, 2023
@bpasero bpasero added bug Issue identified by VS Code Team member as probable bug notebook labels Feb 3, 2023
@roblourens
Copy link
Member

I guess

  1. Why is it forcing a save?
  2. And if I cancel the save, why doesn't that cancel reloading the EH?

@bpasero
Copy link
Member

bpasero commented Feb 4, 2023

@roblourens because all notebooks get disposed:

dispose(): void {
this._disposables.dispose();
dispose(this._notebookSerializer.values());
}

Which triggers closing:

// CLOSE notebook editor for models that have no more serializer
this._disposables.add(notebookService.onWillRemoveViewType(e => {
for (const group of editorGroups.groups) {
const staleInputs = group.editors.filter(input => input instanceof NotebookEditorInput && input.viewType === e);
group.closeEditors(staleInputs);
}
}));

We cannot cancel restart of the EH today through any user interaction and to be fair, restarting the EH is somewhat silly still and just to support deprecated API for the root path...

@roblourens
Copy link
Member

Yeah, I just would have to read more to follow how this is different than closing vscode where hot exit works. But true, we still have the huge thread at #69335 and I think that getting rid of restarting the EH is the right fix for this situation.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 23, 2023
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 notebook
Projects
None yet
Development

No branches or pull requests

4 participants