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 2 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
2 changes: 2 additions & 0 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
Expand Up @@ -71,6 +71,8 @@ function App() {
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
36 changes: 30 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,43 @@
// SPDX-License-Identifier: MIT
// Copyright contributors to the kepler.gl project

import React from 'react';
import ReactDOM from 'react-dom';
import React, {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 = () => (
// 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
const root = createRoot(ele);
root.render(
<Provider store={store}>
<DataLoader store={store} onRenderComplete={onRenderComplete} />
<App />
</Provider>
);
Comment thread
yharby marked this conversation as resolved.

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);