Migrate datalayer specific components from jupyter-react to core #389

goanpeca · 2025-08-13T16:54:50Z

No description provided.

…and kernels!

Copilot

Pull Request Overview

This PR migrates datalayer-specific components from the jupyter-react package to create a more generic, core package. The changes remove datalayer-specific functionality while introducing a new pluggable collaboration provider system and modernizing various component interfaces.

Removes datalayer-specific configuration, providers, and collaboration logic
Introduces a new pluggable collaboration provider architecture with ICollaborationProvider interface
Renames components and interfaces to remove "Datalayer" branding for better genericity

Reviewed Changes

Copilot reviewed 46 out of 46 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
packages/react/src/state/IDatalayerConfig.ts	Removed datalayer-specific configuration interface
packages/react/src/providers/kernels/DatalayerKernels.ts	Removed datalayer kernel provider implementation
packages/react/src/jupyter/collaboration/providers/JupyterCollaborationProvider.ts	Added new Jupyter collaboration provider implementation
packages/react/src/jupyter/collaboration/providers/NoOpCollaborationProvider.ts	Added no-op collaboration provider for disabled collaboration
packages/react/src/jupyter/collaboration/ICollaborationProvider.ts	Added collaboration provider interface and base class
packages/react/src/jupyter/collaboration/CollaborationContext.tsx	Added React context for collaboration providers
packages/react/src/components/notebook/NotebookExtensions.ts	Renamed interfaces from "Datalayer" to generic naming
packages/react/src/components/notebook/Notebook2.tsx	Updated to use new collaboration provider system
packages/react/src/components/notebook/Notebook.tsx	Migrated to new collaboration provider architecture
packages/react/src/components/codemirror/CodeMirrorEditor.tsx	Renamed component and added backward compatibility alias

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-08-18T06:19:45Z

packages/react/src/state/JupyterReactState.ts

@@ -124,8 +106,8 @@ export function useJupyterReactStoreFromProps(
  const {
    defaultKernelName = DEFAULT_KERNEL_NAME,
    initCode = '',
-    jupyterServerToken = props.serviceManager?.serverSettings.token ?? '',
-    jupyterServerUrl = props.serviceManager?.serverSettings.baseUrl ?? '',
+    jupyterServerToken = props.serviceManager?.serverSettings.token,


The fallback to empty string has been removed, which could cause issues if token is undefined. Consider adding ?? '' to maintain backward compatibility.

Suggested change

jupyterServerToken = props.serviceManager?.serverSettings.token,

jupyterServerToken = props.serviceManager?.serverSettings.token ?? '',

Copilot · 2025-08-18T06:19:46Z

packages/react/src/state/JupyterReactState.ts

-    jupyterServerToken = props.serviceManager?.serverSettings.token ?? '',
-    jupyterServerUrl = props.serviceManager?.serverSettings.baseUrl ?? '',
+    jupyterServerToken = props.serviceManager?.serverSettings.token,
+    jupyterServerUrl = props.serviceManager?.serverSettings.baseUrl,


The fallback to empty string has been removed, which could cause issues if baseUrl is undefined. Consider adding ?? '' to maintain backward compatibility.

Suggested change

jupyterServerUrl = props.serviceManager?.serverSettings.baseUrl,

jupyterServerUrl = props.serviceManager?.serverSettings.baseUrl ?? '',

Copilot · 2025-08-18T06:19:46Z

packages/react/src/jupyter/collaboration/providers/JupyterCollaborationProvider.ts

+    // Handle session expiration (code 4002)
+    if (event.code === 4002) {
+      console.warn('Jupyter collaboration session expired');
+      // Attempt to reconnect could be implemented here?


[nitpick] The comment suggests reconnection could be implemented but doesn't explain why it's not. Consider documenting the decision or implementing automatic reconnection.

Suggested change

// Attempt to reconnect could be implemented here?

// Automatic reconnection is not implemented here to avoid unintended session renewal.

// This decision is based on backend limitations and to ensure users are aware of session expiration.

// If reconnection is desired, it should be handled explicitly by the application or user.

Copilot · 2025-08-18T06:19:46Z

packages/react/src/components/lumino/Lumino.tsx

@@ -17,21 +17,56 @@ export const Lumino = (props: LuminoProps) => {
  const ref = useRef<HTMLDivElement>(null);
  const { children, id, height } = props;
  useEffect(() => {
-    if (ref && ref.current) {
+    console.log(


Multiple console.log statements have been added which should be removed or replaced with proper debug logging in production code.

Copilot · 2025-08-18T06:19:46Z

packages/react/src/components/notebook/Notebook.tsx

+    isLoading,
+    'adapter.panel:',
+    adapter?.panel
+  );



Debug console.log statements should be removed from production code.

Suggested change

Copilot · 2025-08-18T06:19:47Z

packages/react/src/jupyter/JupyterConfig.ts

+    setJupyterServerUrl(
+      jupyterServerUrl ??
+        jupyterConfig.baseUrl ??
+        location.protocol + '//' + location.host + jupyterConfig.baseUrl


This line uses jupyterConfig.baseUrl but the next line constructs a URL using jupyterConfig.baseUrl again, which could be redundant or incorrect. The logic appears to have been simplified incorrectly.

Suggested change

location.protocol + '//' + location.host + jupyterConfig.baseUrl

location.protocol + '//' + location.host + DEFAULT_API_KERNEL_PREFIX_URL

Copilot · 2025-08-18T06:19:47Z

packages/react/src/components/notebook/Notebook2Base.tsx

-            'json',
-            'notebook',
+          // Connect to the collaboration provider
+          await collaborationProvider.connect(sharedModel, id, {


The connect call passes serviceManager and path in options but the id parameter should be the document ID, not the notebook ID. Consider using a more specific document identifier.

Suggested change

await collaborationProvider.connect(sharedModel, id, {

await collaborationProvider.connect(sharedModel, path, {

Copilot · 2025-08-18T06:19:47Z

packages/react/webpack.config.js

-  // './src/examples/NotebookKernelChange';
-  // './src/examples/NotebookLess';
-  './src/examples/NotebookLite';
+  './src/examples/Notebook2Collaborative';


[nitpick] The new entry point is uncommented while the old one is commented out. Consider using a more systematic approach to manage webpack entries or documenting why this specific example should be active.

echarles

LGTM Awesome @goanpeca

goanpeca force-pushed the enh/migration branch from eb95756 to e362d8d Compare August 18, 2025 05:22

goanpeca self-assigned this Aug 18, 2025

Migrate datalayer specific code to core, update collaboration, state …

1291b38

…and kernels!

goanpeca force-pushed the enh/migration branch from e362d8d to 1291b38 Compare August 18, 2025 06:11

goanpeca assigned echarles Aug 18, 2025

goanpeca marked this pull request as ready for review August 18, 2025 06:17

Copilot AI review requested due to automatic review settings August 18, 2025 06:17

Copilot AI reviewed Aug 18, 2025

View reviewed changes

echarles approved these changes Aug 18, 2025

View reviewed changes

echarles merged commit a099c3a into datalayer:main Aug 18, 2025
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Migrate datalayer specific components from jupyter-react to core #389

Migrate datalayer specific components from jupyter-react to core #389

goanpeca commented Aug 13, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Aug 18, 2025

Uh oh!

Copilot AI Aug 18, 2025

Uh oh!

Copilot AI Aug 18, 2025

Uh oh!

Copilot AI Aug 18, 2025

Uh oh!

Copilot AI Aug 18, 2025

Uh oh!

Copilot AI Aug 18, 2025

Uh oh!

Copilot AI Aug 18, 2025

Uh oh!

Copilot AI Aug 18, 2025

Uh oh!

echarles left a comment

Uh oh!

Uh oh!

Uh oh!

	jupyterServerToken = props.serviceManager?.serverSettings.token,
	jupyterServerToken = props.serviceManager?.serverSettings.token ?? '',

	jupyterServerUrl = props.serviceManager?.serverSettings.baseUrl,
	jupyterServerUrl = props.serviceManager?.serverSettings.baseUrl ?? '',

-      // Attempt to reconnect could be implemented here?
+      // Automatic reconnection is not implemented here to avoid unintended session renewal.
+      // This decision is based on backend limitations and to ensure users are aware of session expiration.
+      // If reconnection is desired, it should be handled explicitly by the application or user.

	location.protocol + '//' + location.host + jupyterConfig.baseUrl
	location.protocol + '//' + location.host + DEFAULT_API_KERNEL_PREFIX_URL

	await collaborationProvider.connect(sharedModel, id, {
	await collaborationProvider.connect(sharedModel, path, {

Uh oh!

Migrate datalayer specific components from jupyter-react to core #389

Migrate datalayer specific components from jupyter-react to core #389

Conversation

goanpeca commented Aug 13, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

echarles left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!