Skip to content

refactor: switch use-resize-observer for a custom hook#3579

Open
NexPB wants to merge 9 commits intoShopify:mainfrom
NexPB:lorenzo/change-use-resize-observer
Open

refactor: switch use-resize-observer for a custom hook#3579
NexPB wants to merge 9 commits intoShopify:mainfrom
NexPB:lorenzo/change-use-resize-observer

Conversation

@NexPB
Copy link
Copy Markdown

@NexPB NexPB commented Mar 13, 2026

WHY are these changes introduced?

(Mostly done to bring awareness to the outstanding issue #3428)

Currently depends on use-resize-observer for dev tooling UI, removed use-resize-observer third-party dependency and replaced it with a custom useResizeObserver hook using native browser APIs.

WHAT is this pull request doing?

  • Remove use-resize-observer and replace with custom hook in the @shopify/hydrogen package
  • Update RequestDetails and FlameChartWrapper to import new useResizeObserver
  • Update the Hydrogen Vite plugin optimizeDeps list to remove use-resize-observer

HOW to test your changes?

  • From the repo root, install and build as usual (if needed):
    • pnpm install
  • Run the Hydrogen tests:
    • pnpm --filter @shopify/hydrogen test

Post-merge steps

None.

Checklist

  • I've read the Contributing Guidelines
  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've added a changeset if this PR contains user-facing or noteworthy changes
  • I've added tests to cover my changes
  • I've added or updated the documentation

@NexPB NexPB requested a review from a team as a code owner March 13, 2026 06:38
@NexPB
Copy link
Copy Markdown
Author

NexPB commented Mar 13, 2026

I did notice comments at usehooks-ts mentioning that the package feels/is "abandoned", maybe its best to not rely on any pkg and just check in our own file (as the code for this is minimal).

@NexPB NexPB requested a review from fredericoo March 16, 2026 23:40
@NexPB NexPB changed the title refactor: switch use-resize-observer for usehooks-ts refactor: switch use-resize-observer for a custom hook Mar 16, 2026
@fredericoo
Copy link
Copy Markdown
Contributor

fredericoo commented Mar 17, 2026

the CI failure is because the virtual-routes build pipeline assumes all source files are .tsx. your useResizeObserver.ts is the first .ts file in that directory (correctly .ts since there's no JSX), and three places in the build config only handle .tsx:

  1. CLI tsup entry glob (packages/cli/tsup.config.ts) - **/*.tsx skips your .ts file
  2. Hydrogen tsup entry glob (packages/hydrogen/tsup.config.ts) - same thing
  3. Build-check script (packages/cli/scripts/build-check.mjs) - .replace('.tsx', '.jsx') is a no-op on .ts files, so it looks for useResizeObserver.ts in dist which doesn't exist

fix is three one-liners - widen the globs to **/*.{ts,tsx} and use a regex for the extension mapping. I've pushed it to fb-fix-virtual-routes-ts-build if you want to cherry-pick the top commit.

cheers!

The virtual-routes build pipeline assumed all source files were .tsx.
The new useResizeObserver.ts hook (correctly .ts — no JSX) was excluded
from tsup entry globs and the build-check extension mapping couldn't
resolve it.

- Widen tsup entry globs from `**/*.tsx` to `**/*.{ts,tsx}` in both
  CLI and Hydrogen configs
- Fix build-check.mjs to map both .ts and .tsx to .jsx using a regex,
  and remove the dead .replace('src', 'dist') call

(cherry picked from commit 30002f2)
@NexPB
Copy link
Copy Markdown
Author

NexPB commented Mar 17, 2026

the CI failure is because the virtual-routes build pipeline assumes all source files are .tsx. your useResizeObserver.ts is the first .ts file in that directory (correctly .ts since there's no JSX), and three places in the build config only handle .tsx:

  1. CLI tsup entry glob (packages/cli/tsup.config.ts) - **/*.tsx skips your .ts file
  2. Hydrogen tsup entry glob (packages/hydrogen/tsup.config.ts) - same thing
  3. Build-check script (packages/cli/scripts/build-check.mjs) - .replace('.tsx', '.jsx') is a no-op on .ts files, so it looks for useResizeObserver.ts in dist which doesn't exist

fix is three one-liners - widen the globs to **/*.{ts,tsx} and use a regex for the extension mapping. I've pushed it to fb-fix-virtual-routes-ts-build if you want to cherry-pick the top commit.

cheers!

Cherry picked, thanks!

Merge main into branch to resolve pnpm-lock.yaml conflict.
Regenerated lockfile via pnpm install.
Updated .husky/pre-commit to use pnpx/pnpm (npx/npm are deprecated in Shopify's dev environment).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@fredericoo fredericoo left a comment

Choose a reason for hiding this comment

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

nice work removing the third-party dep - using the native ResizeObserver API directly is much cleaner. couple of things to sort before merging though.

@fredericoo
Copy link
Copy Markdown
Contributor

non-blocking: use-resize-observer is still listed as a dependency in packages/cli/package.json (line 62). Nothing in the CLI imports it anymore since the virtual routes now use the custom hook, so it's dead weight. Let's remove it while we're here, yeah?

NexPB and others added 3 commits April 8, 2026 21:47
…tWrapper.tsx

Co-authored-by: ✦ freddie <45042736+fredericoo@users.noreply.github.com>
…tails.tsx

Co-authored-by: ✦ freddie <45042736+fredericoo@users.noreply.github.com>
@NexPB NexPB requested a review from fredericoo April 8, 2026 13:08
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