Skip to content

Commit

Permalink
Fix React warning with useGeometry deps array
Browse files Browse the repository at this point in the history
  • Loading branch information
axelboc committed Aug 1, 2024
1 parent e6797ca commit 7476afb
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 36 deletions.
10 changes: 7 additions & 3 deletions apps/storybook/src/Utilities.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -378,20 +378,24 @@ useGeometry<
>(
Ctor: new (len: number) => H5WebGeometry<AttributeNames, Params>,
dataLength: number,
params: Params | false | undefined, // skip updates by passing `false` or `undefined`
isInteractive = false, // `true` to recompute bounding sphere on every update for raycasting purposes
params: Params,
config?: {
skipUpdates?: boolean; // set to `true` when R3F object is hidden
isInteractive?: boolean; // set to `true` to keep bounding sphere up to date for raycaster
},
): H5WebGeometry<AttributeNames, Params>

const lineGeometry = useGeometry(
LineGeometry,
ordinates.length,
visible && {
{
abscissas,
ordinates,
abscissaScale,
ordinateScale,
ignoreValue,
},
{ skipUpdates: !visible }
);
```

Expand Down
70 changes: 43 additions & 27 deletions packages/lib/src/vis/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,37 +108,53 @@ export function useGeometry<
>(
Ctor: new (len: number) => H5WebGeometry<AttributeNames, Params>,
dataLength: number,
params: Params | false | undefined, // skip updates by passing `false` or `undefined`
isInteractive = false, // keep bounding sphere up to date for raycaster
params: Params,
config: {
skipUpdates?: boolean; // set to `true` when R3F object is hidden
isInteractive?: boolean; // set to `true` to keep bounding sphere up to date for raycaster
} = {},
): H5WebGeometry<AttributeNames, Params> {
const { skipUpdates = false, isInteractive = false } = config;

const geometry = useMemo(() => new Ctor(dataLength), [Ctor, dataLength]);
const invalidate = useThree((state) => state.invalidate);

useLayoutEffect(() => {
if (!params) {
return;
}

geometry.prepare(params);

for (let i = 0; i < dataLength; i += 1) {
geometry.update(i);
}

if (isInteractive) {
geometry.computeBoundingSphere(); // https://github.com/mrdoob/three.js/issues/1170#issuecomment-3617180
}

Object.values<BufferAttribute>(geometry.attributes).forEach((attr) => {
attr.needsUpdate = true;
});

if (geometry.index) {
geometry.index.needsUpdate = true;
}

invalidate();
}, [geometry, ...Object.values(params || {}), invalidate]); // eslint-disable-line react-hooks/exhaustive-deps
useLayoutEffect(
() => {
if (skipUpdates) {
return;
}

geometry.prepare(params);

for (let i = 0; i < dataLength; i += 1) {
geometry.update(i);
}

if (isInteractive) {
geometry.computeBoundingSphere(); // https://github.com/mrdoob/three.js/issues/1170#issuecomment-3617180
}

Object.values<BufferAttribute>(geometry.attributes).forEach((attr) => {
attr.needsUpdate = true;
});

if (geometry.index) {
geometry.index.needsUpdate = true;
}

invalidate();
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[
geometry,
dataLength,
...Object.values(params), // eslint-disable-line react-hooks/exhaustive-deps
skipUpdates,
isInteractive,
invalidate,
],
);

return geometry;
}
4 changes: 3 additions & 1 deletion packages/lib/src/vis/line/ErrorBars.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function ErrorBars(props: Props) {
const { abscissas, ordinates, errors, color, visible, ignoreValue } = props;
const { abscissaScale, ordinateScale } = useVisCanvasContext();

const geometryParams = visible && {
const geometryParams = {
abscissas,
ordinates,
errors,
Expand All @@ -33,12 +33,14 @@ function ErrorBars(props: Props) {
ErrorBarsGeometry,
ordinates.length,
geometryParams,
{ skipUpdates: !visible },
);

const capsGeometry = useGeometry(
ErrorCapsGeometry,
ordinates.length,
geometryParams,
{ skipUpdates: !visible },
);

return (
Expand Down
7 changes: 5 additions & 2 deletions packages/lib/src/vis/line/Glyphs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,17 @@ function Glyphs(props: Props) {
const geometry = useGeometry(
GlyphsGeometry,
ordinates.length,
visible && {
{
abscissas,
ordinates,
abscissaScale,
ordinateScale,
ignoreValue,
},
hasR3FEventHandlers(pointsProps),
{
skipUpdates: !visible,
isInteractive: hasR3FEventHandlers(pointsProps),
},
);

return (
Expand Down
7 changes: 5 additions & 2 deletions packages/lib/src/vis/line/Line.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,17 @@ function Line(props: Props) {
const geometry = useGeometry(
LineGeometry,
ordinates.length,
visible && {
{
abscissas,
ordinates,
abscissaScale,
ordinateScale,
ignoreValue,
},
hasR3FEventHandlers(lineProps),
{
skipUpdates: !visible,
isInteractive: hasR3FEventHandlers(lineProps),
},
);

return (
Expand Down
2 changes: 1 addition & 1 deletion packages/lib/src/vis/scatter/ScatterPoints.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ function ScatterPoints(props: Props) {
colorScale,
interpolator,
},
true,
{ isInteractive: true },
);

return (
Expand Down

0 comments on commit 7476afb

Please sign in to comment.