Skip to content
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.

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

Certain Vite HMR updates break when the components index.ts file passes a certain threshold of exports #2768

Closed
jeremyagabriel opened this issue Feb 20, 2025 · 8 comments

Comments

@jeremyagabriel
Copy link

jeremyagabriel commented Feb 20, 2025

What is the location of your example repository?

https://github.com/jeremyagabriel/hydrogen-demo-store

Which package or tool is having this issue?

Hydrogen

What version of that package or tool are you using?

2025.1.1

What version of Remix are you using?

2.15.3

Steps to Reproduce

  1. In terminal, clone and enter example repository, npm install, then run locally via npm run dev. Ensure the terminal and browser console are visible
  2. On the homepage in the browser, click a product item to navigate to its PDP. Also, observe the metafields console log with a single entry in the object
  3. In the app/lib/constants/product.ts file, comment in the second item in the PRODUCT_METAFIELDS_IDENTIFIERS array
  4. Save file and observe the [vite] hmr update and following [Error: internal error] in the terminal. If there is NO error at this step, see * below
  5. Refresh the browser page and observe the object for the metafields console log logs with only one entry in the object, implying the file change did not register. If the update DID register see *** below
  6. Restart localhost, refresh the PDP, and see the metafields console log is finally updated, implying the change only occurs after a localhost restart
  7. Kill localhost, then in the app/components/index.ts file, comment out the last active Product export, e.g. export {Product5} from './Product5'; or whatever export was below the threshold needed to break HMR with the * footnote below
  8. Start localhost again, reload PDP, then comment in the next item in the PRODUCT_METAFIELDS_IDENTIFIERS array in app/lib/constants/product.ts and save
  9. Observe there is no [Error: internal error] in the terminal, and the metafields console log will reflect the additional entry either upon save OR after page refresh
  10. Repeat the steps to re-observe the error. For debugging purposes, it is recommended to restart localhost between each intended save. See NOTE below.

*If for some reason there is no [Error: internal error] at this step, kill localhost, then comment in an additional Product component export in the app/components/index.ts file and comment back out the line from step 3. Restart localhost and repeat step 3. If again there is no [Error: internal error] add another Product export, repeat the steps in this footnote.

***Sometimes a [Error: internal error] will still apply the intended changes, thus you will see the intended metafields update. You can comment out that line again in the PRODUCT_METAFIELDS_IDENTIFIERS array, save, restart localhost, and repeat step 3. If the intended change still applies with [Error: internal error], you can technically move on, as the error still occurred and is the crux of the issue.

NOTE: [Error: internal error] may sometimes only log alongside the first erroneous [vite] hmr update with a fresh localhost, where subsequent [vite] hmr update in the same localhost session may not have the error logged, but will likely still have a broken HMR. Restart localhost if you want to ensure [Error: internal error] in the erroneous log

Expected Behavior

In a non-Hydrogen Remix Vite app, where there is no virutal: server, this particular save (from the reproduction steps) would simply result in [vite] page reload in the terminal where HMR doesn't apply but the changes are still registered after a page reload. As I understand it, the virtual: server with the Hydrogen template doesn't apply a [vite] page reload for this kind of save, but instead applies [vite] hmr update, but with a list of the codebase's files, but the changes still apply nonetheless. I'm unclear why the list of files is part of the hmr update log, but assuming it's part of the workaround in lieu of [vite] page reload with the virtual: server?

Actual Behavior

Once the index.ts file for /components reaches a certain threshold of exports, the [vite] hmr update for these kinds of saves will be followed by [Error: internal error] where the change in the file does not apply and you must restart localhost. Note, the supposed threshold isn't the number of exports, rather the culmination of the number of files between all the exports.

Note: with this test repo, certain circumstances even with the [Error: internal error], intended changes can still be observed. In practice with our online stores, the changes are not observed and we must constantly restart localhost after every one of these kinds of saves, which can make the dev experience unusable.

Example hmr error in terminal

@wizardlyhel
Copy link
Contributor

At first, I thought it might be the index barrel exports you have with the components folder .. but I couldn't reproduce the [vite] hmr update output you have where it listed all the files in the project.

I'll need a more simple, reproducible project that cause the multiple files listed in that hmr update output (It doesn't have to error out).

The fact that ./server.ts is part of hmr update with just an edit to the /components/index.tsx seems something is wrong with the project setup

@jeremyagabriel
Copy link
Author

jeremyagabriel commented Feb 21, 2025

@wizardlyhel If you then comment in all the export {Product#} from './Product#: exports in app/components/index.ts do you not observe [Error: internal error] in the terminal upon saving in the file in the repro steps?

export {Product, ProductDraftMediaOverlay} from './Product';
export {Product2} from './Product2';
export {Product3} from './Product3';
export {Product4} from './Product4';
export {Product5} from './Product5';
export {Product6} from './Product6';
export {Product7} from './Product7';
export {Product8} from './Product8';
export {Product9} from './Product9';
export {Product10} from './Product10';
export {Product11} from './Product11';
export {Product12} from './Product12';
export {Product13} from './Product13';
export {Product14} from './Product14';
export {Product15} from './Product15';
export {Product16} from './Product16';
export {Product17} from './Product17';
export {Product18} from './Product18';
export {Product19} from './Product19';
export {Product20} from './Product20';

I would figure there would be nothing wrong with the project setup as this was fine with the Classic Remix Compiler, and is also fine in a non-Hydrogen Remix-Vite application, where server.ts doesn't have virtual:@remix-run/dev/server-build.

I'll create a simpler project that outlines an hmr update that lists all files, even if it doesn't error out. I'll work off the demo store.

@jeremyagabriel
Copy link
Author

As reference, with the Hydrogen demo store as is, if you go into app/data/fragments.ts and just delete, say publishedAt from PRODUCT_CARD_FRAGMENT, you will get an hmr update starting with ./server.ts, followed by hmr invalidate. When in a Remix Vite store not utilizing the virtual:remix/server-build, this particular save would simply result in a [vite] page reload instead, in lieu of hmr I believe.

hmr with server..ts

@wizardlyhel
Copy link
Contributor

wizardlyhel commented Feb 21, 2025

Okay I was right about the index barrel file.

If you change how you import your component file, it would fix this issue.

// app/routes/($locale).products.$handle.tsx

- import {Product} from '~/components/;
+ import {Product} from '~/components/Product'; 

I'm guessing you have hit some hmr limit. I'm not sure where's the limit is that is causing the error.

@jeremyagabriel
Copy link
Author

@wizardlyhel So the Hydrogen + Vite solution is to no longer have all our components exported from just the ~/components folder? I realize this would alleviate the error, but it seems suspicious I cannot replicate the same error with a Remix Vite store without a server.ts. Is this a Vite issue or a Hydrogen + Vite issue? So something feels unstable, and the workaround feels like only a temporary solution, as exporting all our components from a single index has been a pattern in all our stores, whether using Hydrogen and Classic Remix Compiler, or when were using Next.js. This wouldn't change the coding pattern of just one store, we have 10's of stores on or will be on Hydrogen.

@wizardlyhel
Copy link
Contributor

It might be within Hydrogen as we are using workerd (to simulate a Cloudflare worker like local environment). We can look deeper into this but it won't be fast turn around to fix this issue

@jeremyagabriel
Copy link
Author

@wizardlyhel Got it! To the small handful of stores we migrated to Vite, we will migrate back to the Classic Remix Compiler to avoid our devs from experiencing new limiting caveats and dev errors. Migrating to Vite doesn't seem to be a "requirement" for Hydrogen any time soon, rather a nice to have, if I'm understanding its support in Hydrogen correctly. If/when there is further discovery on this and a release, we will migrate to Vite. Thank you!

@jeremyagabriel
Copy link
Author

Ok now I'm contemplating removing the use of barrel files 🤔

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

No branches or pull requests

2 participants