Skip to content

fix(GlassSurface): drop the duplicated resize effect - #1021

Open
noron12234 wants to merge 1 commit into
DavidHDev:mainfrom
noron12234:feat/fix-glasssurface-duplicate-observer
Open

fix(GlassSurface): drop the duplicated resize effect#1021
noron12234 wants to merge 1 commit into
DavidHDev:mainfrom
noron12234:feat/fix-glasssurface-duplicate-observer

Conversation

@noron12234

Copy link
Copy Markdown

The problem

GlassSurface registers the same effect twice. Not similar — byte-identical:

useEffect(() => {
  if (!containerRef.current) return;
  const resizeObserver = new ResizeObserver(() => {
    setTimeout(updateDisplacementMap, 0);
  });
  resizeObserver.observe(containerRef.current);
  return () => { resizeObserver.disconnect(); };
}, []);

useEffect(() => {                       // <- the same block again, verbatim
  if (!containerRef.current) return;
  const resizeObserver = new ResizeObserver(() => {
    setTimeout(updateDisplacementMap, 0);
  });
  resizeObserver.observe(containerRef.current);
  return () => { resizeObserver.disconnect(); };
}, []);

So every instance runs two ResizeObservers on the same element, and each resize regenerates the SVG displacement map twice — that's a feImage data-URI rebuild plus feDisplacementMap attribute writes, done twice for nothing.

Measurement

One <GlassSurface> in a stock Vite React app, with ResizeObserver wrapped to count constructions:

ResizeObservers constructed
current main 2
this PR 1

Still renders identically after the change — same three feDisplacementMap nodes, no console errors, and resizes still update the filter (verified by resizing twice via a width prop).

Scope

All four variants carry the same duplication, so all four are fixed: content, tailwind, ts-default, ts-tailwind. The diff is deletion only — 56 lines removed, nothing added.

Verification

  • npx tsc --noEmit: no new errors (zero mentioning GlassSurface).
  • npx prettier --check clean on all four files.
  • npx vite build passes.
  • Browser check above, before and after.

Found while scanning for effects that create a resource without releasing it; this one turned up because the duplicate made the same file report the same pattern twice.

The component registers the same effect twice — byte-identical bodies, both
creating a ResizeObserver on containerRef and calling updateDisplacementMap.
So every instance runs two observers on one element and rebuilds the SVG
displacement map twice on every resize.

Removes the second copy. All four variants carry the same duplication.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request removes a byte-identical, duplicated ResizeObserver useEffect from the GlassSurface component implementations so each instance registers only one observer and performs a single displacement-map regeneration per resize.

Changes:

  • Removed the duplicated ResizeObserver effect in the TS + Tailwind variant.
  • Removed the duplicated ResizeObserver effect in the TS + CSS variant.
  • Removed the duplicated ResizeObserver effect in the JS + Tailwind and JS + CSS variants.

Reviewed changes

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

File Description
src/ts-tailwind/Components/GlassSurface/GlassSurface.tsx Removes the duplicated ResizeObserver useEffect block.
src/ts-default/Components/GlassSurface/GlassSurface.tsx Removes the duplicated ResizeObserver useEffect block.
src/tailwind/Components/GlassSurface/GlassSurface.jsx Removes the duplicated ResizeObserver useEffect block.
src/content/Components/GlassSurface/GlassSurface.jsx Removes the duplicated ResizeObserver useEffect block.

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

Comment on lines 182 to 184
useEffect(() => {
setTimeout(updateDisplacementMap, 0);
}, [width, height]);
Comment on lines 160 to 162
useEffect(() => {
setTimeout(updateDisplacementMap, 0);
}, [width, height]);
Comment on lines 138 to 140
useEffect(() => {
setTimeout(updateDisplacementMap, 0);
}, [width, height]);
Comment on lines 120 to 122
useEffect(() => {
setTimeout(updateDisplacementMap, 0);
}, [width, height]);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants