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

Hydrogen behaves differently between dev monorepo and a standalone app #43

Closed
jplhomer opened this issue Sep 24, 2021 · 1 comment
Closed
Assignees

Comments

@jplhomer
Copy link
Contributor

This is a massive bummer.

We see completely different behavior for bundling the Hydrogen SDK during yarn dev mode:

  • Vite pre-bundles @shopify/hydrogen/client in standalone mode, but not in monorepo mode.
  • This is because, via its support for monorepos, considers other packages in the monorepo to be like local code.
  • This is the most egregious culprit when it comes to shipping bugs: we need to spin up a local project, manually install the latest Hydrogen dev code, and start a dev server before shipping each change to verify nothing broke. This is a step most of us forget to do.
  • Typical bugs include: dependencies getting loaded in the client bundle that ought not be, duplicate context modules getting loaded, etc. See Script component hydrogen#38 for an example.

How can we fix this?

Ideas

optimizeDep.exclude Hydrogen SDK

  • Pros: consistent behavior between monorepo and dev repo, don't have to do any linking etc
  • Cons: have to explicitly optimizeDep.include sub-dependencies, more files

Switch from yarn v1 to something else

Yarn v1 doesn't allow us to use yarn link since Hydrogen is namespaced with @shopify.

Vite just switched to pnpm. vitejs/vite#5060

  • Pros: We could use yarn link probably, get rid of dev workflow and instead prompt devs working on Hydrogen to spin up a local instance that links the Hydrogen build into the repo.
  • Cons: Loss of the easy out-of-the-box Hydrogen developer experience
@frandiox
Copy link
Contributor

frandiox commented Oct 21, 2021

Vite pre-bundles @shopify/hydrogen/client in standalone mode, but not in monorepo mode.
This is because, via its support for monorepos, considers other packages in the monorepo to be like local code.

As mentioned somewhere else, apparently, Vite has a special behavior when it encounters symlinks. And turns out our dev project is basically using a symlink to run Hydrogen.
I could replicate the same behavior in a standalone app when moving <root>/node_modules/@shopify/hydrogen to <root>/hydrogen and creating a symlink with ln -s ../../hydrogen node_modules/@shopify/hydrogen.

I've played a bit with Vite's preserveSymlink option but couldn't find anything interesting.

In any case, the changes in Shopify/hydrogen-archive#683 should bring closer the behavior of Vite in a standalone app to our dev project. However, I'm not sure it fixes all the inconsistencies. We'll need to revisit each issue related to this and try.

optimizeDep.exclude Hydrogen SDK

This seems to be unnecessary after the changes mentioned above, because Vite doesn't need to optimize Hydrogen SDK anymore (since all the CJS dependencies are optimized, and Hydrogen itself is ESM --- or this is what I understand). -- Looks like it now always optimizes hydrogen/client in both standalone and dev, which I guess it's fine?


We could also check pnpm since, at the very least, is way faster than Yarn 1 and is not deprecated. Installing a standalone app in my machine with Yarn takes about 50s, while pnpm is like 10s - 20s? However, I think it still uses symlinks for this kind of thing.

As a last resort, we could have a mechanism that copies shopify/hydrogen/dist/node to shopify/dev/node_modules/@shopify/hydrogen/dist/node or something like that. It's clunky but at least it should work 🤷‍♂️ . It can be manual (yarn refresh && yarn dev) or automatic with some kind of watcher 🤔

Edit: Just remembered about another hacky solution: yarn dev writes to shopify/node_modules/@shopify/hydrogen/dist direclty instead of shopify/hydrogen/dist. I think the dev project will be able to access that thanks to workspaces, and this is not a symlink anymore. I'm using something like this here 😅

@jplhomer jplhomer transferred this issue from another repository Nov 5, 2021
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