diff --git a/packages/monaco/src/browser/monaco-text-model-service.ts b/packages/monaco/src/browser/monaco-text-model-service.ts index 318de5575ba9e..0dc538054bbe0 100644 --- a/packages/monaco/src/browser/monaco-text-model-service.ts +++ b/packages/monaco/src/browser/monaco-text-model-service.ts @@ -113,7 +113,7 @@ export class MonacoTextModelService implements ITextModelService { * creates a model which is not saved by the model service. * this will therefore also not be created on backend side. */ - createUnmangedModel(raw: monaco.Uri | URI): Promise { + createUnmanagedModel(raw: monaco.Uri | URI): Promise { return this.loadModel(new URI(raw.toString())); } diff --git a/packages/notebook/src/browser/service/notebook-monaco-text-model-service.ts b/packages/notebook/src/browser/service/notebook-monaco-text-model-service.ts index 6b0c5e8576206..3d43a4bb5988b 100644 --- a/packages/notebook/src/browser/service/notebook-monaco-text-model-service.ts +++ b/packages/notebook/src/browser/service/notebook-monaco-text-model-service.ts @@ -31,7 +31,7 @@ export class NotebookMonacoTextModelService { protected readonly monacoTextModelService: MonacoTextModelService; protected readonly cellmodels = new ReferenceCollection( - uri => this.monacoTextModelService.createUnmangedModel(new URI(uri)) + uri => this.monacoTextModelService.createUnmanagedModel(new URI(uri)) ); getOrCreateNotebookCellModelReference(uri: URI): Promise> { @@ -39,7 +39,7 @@ export class NotebookMonacoTextModelService { } async createTextModelsForNotebook(notebook: NotebookModel): Promise { - await Promise.all(notebook.cells.map(cell => this.getOrCreateNotebookCellModelReference(cell.uri))); + await Promise.all(notebook.cells.map(cell => cell.resolveTextModel())); } get onDidCreateNotebookCellModel(): Event { diff --git a/packages/notebook/src/browser/service/notebook-service.ts b/packages/notebook/src/browser/service/notebook-service.ts index 4b9b1ba3b5a2d..78795e34d0f9f 100644 --- a/packages/notebook/src/browser/service/notebook-service.ts +++ b/packages/notebook/src/browser/service/notebook-service.ts @@ -119,7 +119,7 @@ export class NotebookService implements Disposable { this.notebookModels.set(resource.uri.toString(), model); // Resolve cell text models right after creating the notebook model // This ensures that all text models are available in the plugin host - this.textModelService.createTextModelsForNotebook(model); + await this.textModelService.createTextModelsForNotebook(model); this.didAddNotebookDocumentEmitter.fire(model); model.onDidDispose(() => { this.notebookModels.delete(resource.uri.toString()); diff --git a/packages/notebook/src/browser/view-model/notebook-cell-model.ts b/packages/notebook/src/browser/view-model/notebook-cell-model.ts index 303d075e0cd2a..038666cd24d21 100644 --- a/packages/notebook/src/browser/view-model/notebook-cell-model.ts +++ b/packages/notebook/src/browser/view-model/notebook-cell-model.ts @@ -264,7 +264,6 @@ export class NotebookCellModel implements NotebookCell, Disposable { this.onDidChangeMetadataEmitter.dispose(); this.onDidChangeInternalMetadataEmitter.dispose(); this.onDidChangeLanguageEmitter.dispose(); - this.textModel?.dispose(); this.toDispose.dispose(); } @@ -356,9 +355,10 @@ export class NotebookCellModel implements NotebookCell, Disposable { const ref = await this.textModelService.getOrCreateNotebookCellModelReference(this.uri); this.textModel = ref.object; - this.textModel.onDidChangeContent(e => { + this.toDispose.push(ref); + this.toDispose.push(this.textModel.onDidChangeContent(e => { this.props.source = e.model.getText(); - }); + })); return ref.object; } diff --git a/packages/notebook/src/browser/view-model/notebook-model.ts b/packages/notebook/src/browser/view-model/notebook-model.ts index 4230666db5e63..ed792866028b3 100644 --- a/packages/notebook/src/browser/view-model/notebook-model.ts +++ b/packages/notebook/src/browser/view-model/notebook-model.ts @@ -331,7 +331,7 @@ export class NotebookModel implements Saveable, Disposable { } - protected async replaceCells(start: number, deleteCount: number, newCells: CellData[], computeUndoRedo: boolean): Promise { + protected replaceCells(start: number, deleteCount: number, newCells: CellData[], computeUndoRedo: boolean): void { const cells = newCells.map(cell => { const handle = this.nextHandle++; return this.cellModelFactory({ @@ -362,10 +362,6 @@ export class NotebookModel implements Saveable, Disposable { async () => this.replaceCells(start, deleteCount, newCells, false)); } - // Ensure that all text model have been created - // Otherwise we run into a race condition once we fire `onDidChangeContent` - await Promise.all(cells.map(cell => cell.resolveTextModel())); - this.onDidAddOrRemoveCellEmitter.fire({ rawEvent: { kind: NotebookCellsChangeType.ModelChange, changes }, newCellIds: cells.map(cell => cell.handle) }); this.onDidChangeContentEmitter.queue({ kind: NotebookCellsChangeType.ModelChange, changes }); if (cells.length > 0) { diff --git a/packages/notebook/src/common/notebook-common.ts b/packages/notebook/src/common/notebook-common.ts index bf7355035c796..58e8fdc8ada9b 100644 --- a/packages/notebook/src/common/notebook-common.ts +++ b/packages/notebook/src/common/notebook-common.ts @@ -273,7 +273,7 @@ export namespace CellUri { const s = handle.toString(_radix); const p = s.length < _lengths.length ? _lengths[s.length - 1] : 'z'; - const fragment = `${p}${s}s${Buffer.from(BinaryBuffer.fromString(notebook.scheme).buffer).toString('base64')} `; + const fragment = `${p}${s}s${Buffer.from(BinaryBuffer.fromString(notebook.scheme).buffer).toString('base64')}`; return notebook.withScheme(cellUriScheme).withFragment(fragment); } diff --git a/packages/plugin-ext/src/plugin/editors-and-documents.ts b/packages/plugin-ext/src/plugin/editors-and-documents.ts index d5a98a9863fa3..23a39b050875e 100644 --- a/packages/plugin-ext/src/plugin/editors-and-documents.ts +++ b/packages/plugin-ext/src/plugin/editors-and-documents.ts @@ -46,6 +46,10 @@ export class EditorsAndDocumentsExtImpl implements EditorsAndDocumentsExt { private readonly editors = new Map(); async $acceptEditorsAndDocumentsDelta(delta: EditorsAndDocumentsDelta): Promise { + this.acceptEditorsAndDocumentsDelta(delta); + } + + acceptEditorsAndDocumentsDelta(delta: EditorsAndDocumentsDelta): void { const removedDocuments = new Array(); const addedDocuments = new Array(); const removedEditors = new Array(); diff --git a/packages/plugin-ext/src/plugin/notebook/notebook-document.ts b/packages/plugin-ext/src/plugin/notebook/notebook-document.ts index 456e218b3d250..edc9a008853d0 100644 --- a/packages/plugin-ext/src/plugin/notebook/notebook-document.ts +++ b/packages/plugin-ext/src/plugin/notebook/notebook-document.ts @@ -27,6 +27,7 @@ import * as typeConverters from '../type-converters'; import { ModelAddedData, NotebookCellDto, NotebookCellsChangedEventDto, NotebookModelAddedData, NotebookOutputDto } from '../../common'; import { NotebookRange } from '../types-impl'; import { DocumentsExtImpl } from '../documents'; +import { UriComponents } from '../../common/uri-components'; class RawContentChangeEvent { @@ -345,6 +346,9 @@ export class NotebookDocument implements Disposable { return; } + const addedDocuments: ModelAddedData[] = []; + const removedDocuments: UriComponents[] = []; + const contentChangeEvents: RawContentChangeEvent[] = []; splices.reverse().forEach(splice => { @@ -353,9 +357,7 @@ export class NotebookDocument implements Disposable { const extCell = new Cell(this, this.editorsAndDocuments, cell); if (!initialization) { - this.editorsAndDocuments.$acceptEditorsAndDocumentsDelta({ - addedDocuments: [Cell.asModelAddData(cell)] - }); + addedDocuments.push(Cell.asModelAddData(cell)); } return extCell; }); @@ -364,10 +366,18 @@ export class NotebookDocument implements Disposable { const deletedItems = this.cells.splice(splice.start, splice.deleteCount, ...newCells); for (const cell of deletedItems) { changeEvent.deletedItems.push(cell.apiCell); + removedDocuments.push(cell.uri.toComponents()); } contentChangeEvents.push(changeEvent); }); + if (addedDocuments.length > 0 || removedDocuments.length > 0) { + this.editorsAndDocuments.acceptEditorsAndDocumentsDelta({ + addedDocuments, + removedDocuments + }); + } + if (bucket) { for (const changeEvent of contentChangeEvents) { bucket.push(changeEvent.asApiEvent()); diff --git a/packages/plugin-ext/src/plugin/notebook/notebooks.ts b/packages/plugin-ext/src/plugin/notebook/notebooks.ts index 58a920e76d992..a618f8ad0222d 100644 --- a/packages/plugin-ext/src/plugin/notebook/notebooks.ts +++ b/packages/plugin-ext/src/plugin/notebook/notebooks.ts @@ -22,7 +22,7 @@ import { CancellationToken, Disposable, DisposableCollection, Emitter, Event, UR import { URI as TheiaURI } from '../types-impl'; import * as theia from '@theia/plugin'; import { - CommandRegistryExt, ModelAddedData, NotebookCellStatusBarListDto, NotebookDataDto, + CommandRegistryExt, NotebookCellStatusBarListDto, NotebookDataDto, NotebookDocumentsAndEditorsDelta, NotebookDocumentShowOptions, NotebookDocumentsMain, NotebookEditorAddData, NotebookEditorsMain, NotebooksExt, NotebooksMain, Plugin, PLUGIN_RPC_CONTEXT } from '../../common'; @@ -205,7 +205,6 @@ export class NotebooksExtImpl implements NotebooksExt { async $acceptDocumentsAndEditorsDelta(delta: NotebookDocumentsAndEditorsDelta): Promise { const removedCellDocuments: UriComponents[] = []; - const addedCellDocuments: ModelAddedData[] = []; if (delta.removedDocuments) { for (const uri of delta.removedDocuments) { const revivedUri = URI.fromComponents(uri); @@ -226,10 +225,12 @@ export class NotebooksExtImpl implements NotebooksExt { } } - // publish all removed cell documents first - await this.textDocumentsAndEditors.$acceptEditorsAndDocumentsDelta({ - removedDocuments: removedCellDocuments - }); + if (removedCellDocuments.length > 0) { + // publish all removed cell documents first + this.textDocumentsAndEditors.acceptEditorsAndDocumentsDelta({ + removedDocuments: removedCellDocuments + }); + } if (delta.addedDocuments) { for (const modelData of delta.addedDocuments) { @@ -250,17 +251,18 @@ export class NotebooksExtImpl implements NotebooksExt { this.documents.get(uri.toString())?.dispose(); this.documents.set(uri.toString(), document); - addedCellDocuments.push(...modelData.cells.map(cell => Cell.asModelAddData(cell))); + if (modelData.cells.length > 0) { + // Publish new cell documents before calling the notebook document open event + // During this event, extensions might request the cell document and we want to make sure it is available + this.textDocumentsAndEditors.acceptEditorsAndDocumentsDelta({ + addedDocuments: modelData.cells.map(cell => Cell.asModelAddData(cell)) + }); + } this.onDidOpenNotebookDocumentEmitter.fire(document.apiNotebook); } } - // publish all added cell documents in a separate call - await this.textDocumentsAndEditors.$acceptEditorsAndDocumentsDelta({ - addedDocuments: addedCellDocuments - }); - if (delta.addedEditors) { for (const editorModelData of delta.addedEditors) { if (this.editors.has(editorModelData.id)) {