Skip to content

fix(jupyter): Fix initial viewport sizing and React 18 migration #3253

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.

Sign up for GitHub

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

Open
yharby wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix(jupyter): Fix initial viewport sizing and React 18 migration #3253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cc5d3b2
fix(jupyter): call handleResize on mount for correct initial viewport…
yharby Dec 3, 2025
e6d70d0
fix(jupyter): migrate to React 18 createRoot API and fix data loading
yharby Dec 3, 2025
58242cc
fix(jupyter): simplify resize logic and add WeakMap for root instances
yharby Dec 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions bindings/kepler.gl-jupyter/js/lib/keplergl/components/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: MIT
// Copyright contributors to the kepler.gl project

import React, {useEffect, useState, useRef} from 'react';
import {useEffect, useState, useRef} from 'react';
import styled from 'styled-components';
const ReactHelmet = require('react-helmet');
const Helmet = ReactHelmet ? ReactHelmet.Helmet : null;
Expand Down Expand Up @@ -58,19 +58,18 @@ function App() {

const width = rootElm.current.offsetWidth;
const height = rootElm.current.offsetHeight;
const dimensionToSet = {
...(width && width !== windowDimension.width ? {width} : {}),
...(height && height !== windowDimension.height ? {height} : {})
};

setDimension(dimensionToSet);
if (width && height) {
setDimension({width, height});

Copilot AI Dec 8, 2025

Copy link

Choose a reason for hiding this comment

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

The dimension update logic has been simplified, but this may cause unnecessary re-renders. The previous implementation only updated state when dimensions actually changed:

const dimensionToSet = {
  ...(width && width !== windowDimension.width ? {width} : {}),
  ...(height && height !== windowDimension.height ? {height} : {})
};
setDimension(dimensionToSet);

The new logic always calls setDimension({width, height}) even if the dimensions haven't changed. Consider adding a check to only update when dimensions actually change:

if ((width && width !== windowDimension.width) || (height && height !== windowDimension.height)) {
  setDimension({width, height});
}
Suggested change
setDimension({width, height});
if (
width !== windowDimension.width ||
height !== windowDimension.height
) {
setDimension({width, height});
}

Copilot uses AI. Check for mistakes.
}
};

// in Jupyter Lab, parent component has transition when window resize.
// in Jupyter Lab, parent component has transition when window resize.
// need to delay call to get the final parent width,
const resizeDelay = () => window.setTimeout(handleResize, 500);

useEffect(() => {
// Call handleResize on mount to set initial dimensions
handleResize();
window.addEventListener('resize', resizeDelay);
return () => window.removeEventListener('resize', resizeDelay);
}, []);
Comment thread
yharby marked this conversation as resolved.
Comment on lines 70 to 75

Copilot AI Dec 8, 2025

Copy link

Choose a reason for hiding this comment

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

The cleanup function in useEffect doesn't clear the timeout created by resizeDelay(). If the component unmounts while a timeout is pending, the timeout callback will still execute, potentially calling handleResize() on an unmounted component.

Consider storing the timeout ID and clearing it in the cleanup:

useEffect(() => {
  handleResize();
  let timeoutId = null;
  const resizeHandler = () => {
    if (timeoutId) clearTimeout(timeoutId);
    timeoutId = window.setTimeout(handleResize, 500);
  };
  window.addEventListener('resize', resizeHandler);
  return () => {
    window.removeEventListener('resize', resizeHandler);
    if (timeoutId) clearTimeout(timeoutId);
  };
}, []);

Copilot uses AI. Check for mistakes.
Expand Down
44 changes: 38 additions & 6 deletions bindings/kepler.gl-jupyter/js/lib/keplergl/components/root.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,51 @@
// SPDX-License-Identifier: MIT
// Copyright contributors to the kepler.gl project

import React from 'react';
import ReactDOM from 'react-dom';
import {useEffect, useRef} from 'react';
import {createRoot} from 'react-dom/client';
import {Provider} from 'react-redux';
import App from './app';
import Window from 'global/window';
import {addDataConfigToKeplerGl} from '../kepler.gl';

function renderRoot({id, store, ele}) {
const Root = () => (
// Store root instances to avoid creating multiple roots for the same element
const rootInstances = new WeakMap();

// Separate component to handle data loading after mount
function DataLoader({store, onRenderComplete}) {
const hasLoadedData = useRef(false);

useEffect(() => {
// This runs AFTER component tree is mounted and Provider is fully subscribed
// Load data from Window.__keplerglDataConfig (HTML export mode)
if (!hasLoadedData.current && Window.__keplerglDataConfig) {
hasLoadedData.current = true;
const {data, config, options} = Window.__keplerglDataConfig;
addDataConfigToKeplerGl({data, config, options, store});
}
// Signal render complete for callback-based loading (Jupyter widget mode)
if (onRenderComplete) {
onRenderComplete();
}
}, [store, onRenderComplete]);
Comment on lines +18 to +30

Copilot AI Dec 8, 2025

Copy link

Choose a reason for hiding this comment

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

Including store in the useEffect dependency array may cause unexpected behavior. If the store reference changes (which shouldn't happen in normal usage), the effect will re-run but hasLoadedData.current will still be true, preventing data from being reloaded with the new store.

Consider either:

  1. Removing store from the dependency array (if the store reference is guaranteed to be stable)
  2. Resetting hasLoadedData.current = false when the store changes
  3. Using a WeakMap to track loaded state per store instance

The current implementation assumes the store reference never changes while also including it in dependencies, which is inconsistent.

Copilot uses AI. Check for mistakes.

return null;
}

function renderRoot({id, store, ele, onRenderComplete}) {
// Use React 18 createRoot API - reuse existing root if available
let root = rootInstances.get(ele);
if (!root) {
root = createRoot(ele);
rootInstances.set(ele, root);
}

root.render(
<Provider store={store}>
<DataLoader store={store} onRenderComplete={onRenderComplete} />
<App />
</Provider>
);

ReactDOM.render(<Root />, ele);
}

export default renderRoot;
9 changes: 2 additions & 7 deletions bindings/kepler.gl-jupyter/js/lib/keplergl/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
import createAppStore from './store';
import renderRoot from './components/root';
import document from 'global/document';
import Window from 'global/window';
import {addDataConfigToKeplerGl} from './kepler.gl';

const map = (function initKeplerGl() {
const id = 'keplergl-0';
Expand All @@ -18,15 +16,12 @@ const map = (function initKeplerGl() {

return {
render: () => {
// Data loading is now handled inside renderRoot via useEffect
// This ensures Redux Provider subscriptions are active before dispatching
renderRoot({id, store, ele: divElmt});
},
store
};
})();

map.render();

(function loadDataConfig(keplerGlMap) {
const {data, config, options} = Window.__keplerglDataConfig || {};
addDataConfigToKeplerGl({data, config, options, store: keplerGlMap.store});
})(map);