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

Ensure 3p developers can properly integrate with Hydrogen #1388

Open
jplhomer opened this issue May 31, 2022 · 6 comments
Open

Ensure 3p developers can properly integrate with Hydrogen #1388

jplhomer opened this issue May 31, 2022 · 6 comments

Comments

@jplhomer
Copy link
Contributor

jplhomer commented May 31, 2022

Originally posted by @susannepeng in Shopify/hydrogen#341 (comment)

Hi all,

We're developing a third party component library for use with Hydrogen and running into the same problem when we try to import anything from @shopify/hydrogen from our component library.

Our component library lists @shopify/hydrogen as a peer dependency and the components it exports are designed to be used by a merchant in their own Hydrogen app.

Demo

The third party component

The third party component used inside of a Hydrogen app

For now, the third party component library is packaged via npm pack and listed as a dependency of the user's Hydrogen app. Both repos linked above were set up to help illustrate this issue.

Errors

When running the app with npm run dev:

image

And the dev server output shows:

@shopify/hydrogen doesn't appear to be written in CJS, but also doesn't appear to be a valid ES module (i.e. it doesn't have "type": "module" or an .mjs extension for the entry point). Please contact the package author to fix.
Sourcemap for "/Users/susannepeng/dev/temp/hydrogen/hydrogen-app-cjs-error-demo/node_modules/kolorist/dist/module/index.js" points to missing source files
2:49:03 pm [vite] Error when evaluating SSR module /src/App.server:
/Users/susannepeng/dev/temp/hydrogen/hydrogen-app-cjs-error-demo/node_modules/@shopify/hydrogen/client.js:1
export * from './dist/esnext/client';
^^^^^^

SyntaxError: Unexpected token 'export'
    at wrapSafe (internal/modules/cjs/loader.js:1001:16)
    at Module._compile (internal/modules/cjs/loader.js:1049:27)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.Module._load (internal/modules/cjs/loader.js:790:12)
    at Module.require (internal/modules/cjs/loader.js:974:19)
    at require (internal/modules/cjs/helpers.js:93:18)
    at Object.<anonymous> (/Users/susannepeng/dev/temp/hydrogen/hydrogen-app-cjs-error-demo/node_modules/third-party-component-library/dist/cjs/components/ThirdPartyServerComponent.server.js:8:18)
    at Module._compile (internal/modules/cjs/loader.js:1085:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)

Re: node module resolution

We think this may be a module resolution issue. Our component library expects to use certain Hydrogen components, hooks, etc.. We compile our modules to both CJS and ESM and the user app resolves to our CJS modules, which are in turn attempting to resolve modules from @shopify/hydrogen. Here we run into the problem -- it seems that certain Hydrogen foundation/framework modules are only compiled to ESM.

Further investigation indicates that @shopify/hydrogen has been packaged this way due to its dependency on Vite. There has also been talk of not compiling to CJS at all.

We tried setting the user app to use ESM module resolution and were met with the following error:

> hydrogen-app-cjs-error-demo@0.0.0 dev /Users/susannepeng/dev/temp/hydrogen/hydrogen-app-cjs-error-demo
> shopify hydrogen dev

failed to load config from /Users/susannepeng/dev/temp/hydrogen/hydrogen-app-cjs-error-demo/vite.config.js

━━━━━━ Error ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

    Cannot find module '/Users/susannepeng/dev/temp/hydrogen/hydrogen-app-cjs-error-demo/node_modules/@shopify/hydrogen/plugin' imported from /Users/susannepeng/dev/temp/hydrogen/hydrogen-app-cjs-error-demo/vite.config.js
Did you mean to import @shopify/hydrogen/plugin.js?

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

We tried to dig further into this but got lost in the weeds of Vite internals.

For compatibility purposes and encouraging the growth of an ecosystem of third party libraries surrounding Hydrogen, could @shopify/hydrogen also provide modules compiled to CJS?

@jplhomer
Copy link
Contributor Author

Originally posted by @frandiox in Shopify/hydrogen#341 (comment)

@susannepeng Hi!

To answer your question, though: Hydrogen is used as ESM in Vite. If you were to import a CJS version of Hydrogen in your library, the user would end up with 2 different copies of that Hydrogen component. Plus, Vite itself and the ecosystem are moving towards ESM-first.

I think the problem here is that your library is imported as CJS, and then it imports Hydrogen as ESM, right? Could you make sure your lib is imported as ESM instead? For that, I think you might need to provide ESM-only version and perhaps play with Vite's optimizeDeps.include.

If that still doesn't work, please report this again as a new issue with your findings.

@jplhomer jplhomer changed the title Ensure 3p developers and properly integrate with Hydrogen Ensure 3p developers can properly integrate with Hydrogen May 31, 2022
@jplhomer
Copy link
Contributor Author

jplhomer commented Jun 1, 2022

Hi @susannepeng, I just pulled down a fresh copy of Owly-Owl/hydrogen-app-cjs-error-demo but it appears to be working properly:

Screen Shot 2022-06-01 at 6 11 12 AM

Is there something else I need to do to reproduce the error?

@susannepeng
Copy link

susannepeng commented Jun 1, 2022

Hi @jplhomer and @frandiox, thanks so much for your responses!

We got our third party component library to work alongside @shopify/hydrogen

We took a similar approach to what @frandiox outlined in this comment which indeed got us over the line in terms of getting our bare bones third party component library loading in successfully alongside @shopify/hydrogen in a merchant app.

You saw the results when you pulled your copy of Owly-Owl/hydrogen-app-cjs-error-demo. Sorry for any confusion here!

We ran into problems when we tried to introduce our own dependencies

We had a second issue when we tried to add dependencies of our own into our component library, however. One particular dependency we were using provided both CJS and ESM versions of its modules, but we noticed that Vite didn't seem to respect its package.json:exports configuration and tried to load its CJS modules as ESM.

Sample error output of the second issue:

[vite] Error when evaluating SSR module /node_modules/html-react-parser/index.js:
ReferenceError: require is not defined
    at /node_modules/html-react-parser/index.js:1:18

I've recorded a short video to illustrate what we found:
https://www.loom.com/share/5919e13da376424fb3d8b9dd18606e2a

To summarise, we got around this by making Vite pre-bundle our third party library's dependency by modifying optimizeDeps.include in the merchant app's vite.config.js like so:

optimizeDeps: {
  include: [
    '@headlessui/react',
    'third-party-component-library > html-react-parser',
  ],
},

We followed the Vite documentation here for the above configuration.

What now?

Although the above works we are unsure if it's ideal for merchants or for third party package authors. Merchants shouldn't need to manage an ever growing/changing list of dependencies of dependencies that need to be pre-bundled to be compatible with Vite. Likewise, third party package authors shouldn't need to document which of their dependencies merchants need to pre-bundle.

Is there a better way? Like I mentioned in the video we are new to working with Vite and ESM-only packages so any direction here would be very welcome!!

The updated code can be found here:

You can comment out the below line in vite.config.js to reproduce the problem:

'third-party-component-library > html-react-parser',

@frandiox
Copy link
Contributor

frandiox commented Jun 2, 2022

Great to see you fixed it and that the pointers were useful!

Is there a better way? Like I mentioned in the video we are new to working with Vite and ESM-only packages so any direction here would be very welcome!!

I'll try to test a bit more with your code and see if there's a better solution but, unfortunately, I think this is the way things need to be in Vite right now. Perhaps this will be different in Vite 3 (already in alpha), where they are switching to ESM default for SSR.

For the time being, you could provide a Vite plugin that simply adds your optimized deps to the config hook. That would be easier than teaching users how to modify optimizeDeps. It would be something like plugins: [hydrogen(), 3plib()] instead.

@frandiox
Copy link
Contributor

frandiox commented Jun 3, 2022

I've checked and creating a plugin still seems to be the simplest way to make it work in Vite. Another way to do it would be bundling all your dependencies at build time (using Vite itself in library mode), but that would duplicate code if another library uses html-react-parser 🤔

@susannepeng
Copy link

Thanks for looking into this @frandiox! The plugin route works nicely. We ended up providing a plugin in very much the same way as the Hydrogen plugin is provided. Cheers 🙂

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

3 participants