fix: editors

[echarles]

[netlify]

❌ Deploy Preview for datalayer-desktop failed.

Name	Link
🔨 Latest commit	499688d
🔍 Latest deploy log	https://app.netlify.com/projects/datalayer-desktop/deploys/699a06e2c0a2880008c2f922

[Copilot]

Pull request overview

This PR addresses editor runtime/module integration issues in the renderer, primarily to improve Jupyter/Lexical editor compatibility in the Electron/Vite environment.

Changes:

• Add a path module polyfill to the RequireJS shim to unblock CommonJS JupyterLab code that does require("path").
• Plumb a serviceManager prop from DocumentEditor into the Lexical editor flow and adjust useJupyter initialization.
• Wrap the Lexical editor with CommentsProvider to support comment-related plugins/UI.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
src/renderer/polyfills/requirejs.js	Registers a path module in the RequireJS registry to prevent crashes in CJS JupyterLab bundles.
src/renderer/pages/DocumentEditor.tsx	Passes a serviceManager into LexicalEditor when rendering the document editor.
src/renderer/components/lexical/LexicalEditor.tsx	Adds serviceManager to props, changes useJupyter initialization, and wraps editor in CommentsProvider.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

serviceManager is accepted in LexicalEditor props but never used, which will trigger @typescript-eslint/no-unused-vars (repo runs eslint with --max-warnings 0). Either forward it to LexicalEditorContainer (so it’s actually used) or rename it to _serviceManager/remove it if not needed.

[Copilot]

LexicalEditorContainer expects a serviceManager prop (propsServiceManager), but LexicalEditor never passes its serviceManager down. As a result, the custom runtime/service manager created in DocumentEditor won’t be used by the Jupyter plugins inside the editor.

Suggested change

	onContentChange={onContentChange}
	onContentChange={onContentChange}
	serviceManager={serviceManager}

[Copilot]

useJupyter previously ran with default options, but now you always pass { startDefaultKernel: !!propsServiceManager }. Callers that don’t provide a service manager (e.g. LexicalDocument renders <LexicalEditor ... /> without serviceManager) will now pass startDefaultKernel: false, which can prevent defaultKernel from ever being created and break JupyterInputOutputPlugin/ComponentPickerMenuPlugin. Preserve the prior default by omitting startDefaultKernel when propsServiceManager is undefined (or default it to true).

Suggested change

	const { serviceManager, defaultKernel } = useJupyter({
	serviceManager: propsServiceManager,
	startDefaultKernel: !!propsServiceManager,
	});
	const jupyterOptions = propsServiceManager
	? { serviceManager: propsServiceManager, startDefaultKernel: true }
	: {};
	const { serviceManager, defaultKernel } = useJupyter(jupyterOptions);

[Copilot]

The new path polyfill’s resolve()/relative() don’t match Node’s path.posix semantics (e.g., relative(from, to) currently ignores from, and resolve() doesn’t normalize ./.. segments). If any Jupyterlab CJS code relies on these, it will produce incorrect paths. Either implement minimal-but-correct posix semantics for the functions you expose, or restrict the polyfill to only the functions you know are needed (and fail loudly for unsupported ones).

Suggested change

	return '/' + paths.filter(p => p).join('/').replace(/\/+/g, '/');
	},
	relative: function (from, to) {
	return to || '';
	// Minimal POSIX-like resolve: process from right to left, stop at first
	// absolute path, then normalize. We treat "/" as the cwd when no absolute
	// segment is provided.
	let resolvedPath = '';
	let isAbsolute = false;
	for (let i = paths.length - 1; i >= 0; i--) {
	let p = paths[i];
	if (!p) continue;
	if (p[0] === '/') {
	resolvedPath = p + '/' + resolvedPath;
	isAbsolute = true;
	break;
	} else {
	resolvedPath = p + '/' + resolvedPath;
	}
	}
	if (!isAbsolute) {
	resolvedPath = '/' + resolvedPath;
	}
	// Use existing normalize implementation to handle "." and ".." segments.
	return this.normalize(resolvedPath);
	},
	relative: function (from, to) {
	// Minimal POSIX-like relative: compute the relative path from "from" to "to"
	// using the resolved, normalized absolute paths.
	from = this.resolve(from || '');
	to = this.resolve(to || '');
	if (from === to) {
	return '';
	}
	const fromParts = from.slice(1).split('/').filter(Boolean);
	const toParts = to.slice(1).split('/').filter(Boolean);
	const length = Math.min(fromParts.length, toParts.length);
	let samePartsLength = length;
	for (let i = 0; i < length; i++) {
	if (fromParts[i] !== toParts[i]) {
	samePartsLength = i;
	break;
	}
	}
	const outputParts = [];
	for (let i = samePartsLength; i < fromParts.length; i++) {
	outputParts.push('..');
	}
	outputParts.push(...toParts.slice(samePartsLength));
	return outputParts.join('/');

[Copilot]

parse() relies on this.dirname/this.basename/..., which will break if consumers ever call it unbound (e.g. const { parse } = require('path'); parse('x')). Also root is always returned as '', but for absolute paths Node returns '/' in path.posix.parse. Use direct references (not this) and align the returned shape with Node’s path.posix.parse to avoid subtle incompatibilities in downstream Jupyterlab code.

Suggested change

	return {
	root: '',
	dir: this.dirname(p),
	base: this.basename(p),
	ext: this.extname(p),
	name: this.basename(p, this.extname(p)),
	};
	// Align with Node's path.posix.parse semantics.
	if (!p) {
	return { root: '', dir: '', base: '', ext: '', name: '' };
	}
	const isAbsolute = p[0] === '/';
	const root = isAbsolute ? '/' : '';
	const dirRaw = pathModule.dirname(p);
	const dir = !isAbsolute && dirRaw === '.' ? '' : dirRaw;
	const base = pathModule.basename(p);
	const ext = pathModule.extname(p);
	const name = pathModule.basename(p, ext);
	return { root, dir, base, ext, name };