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

🐛 BUG: react component + client:load fails with preact/compat but works fine with @astrojs/react #4107

Closed
1 task
mayank99 opened this issue Jul 31, 2022 · 10 comments · Fixed by #4213 or #4267
Closed
1 task
Assignees
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) pkg: preact Related to Preact (scope)

Comments

@mayank99
Copy link
Contributor

What version of astro are you using?

1.0.0-rc.3

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

npm

What operating system are you using?

Windows / Stackblitz

Describe the Bug

I have a ComboBox component from react-aria that I'm trying to render with client:load using the @astrojs/preact adapter with { compat: true }. It gives me the following error:

Unable to render ComboBox!

This component likely uses @astrojs/react, @astrojs/preact, @astrojs/vue
or @astrojs/svelte, but Astro encountered an error during server-side rendering.

Please ensure that ComboBox:
1. Does not unconditionally access browser-specific globals like `window` or `document`.
   If this is unavoidable, use the `client:only` hydration directive.
2. Does not conditionally return `null` or `undefined` when rendered on the server.

Using client:load='preact' works fine so I believe this is an SSR issue. However, I noticed that swapping out preact({ compat: true }) with just react() makes it work with client:load again.

(I did also come across #3306, but this is react-aria not react-spectrum (which is a styled wrapper around react-aria) so there is no CSS involved. This could be why it works fine on server with react().)

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-xkptsr?file=src%2Fpages%2Findex.astro,astro.config.mjs

Participation

  • I am willing to submit a pull request for this issue.
@FredKSchott FredKSchott added - P3: minor bug An edge case that only affects very specific usage (priority) s2-medium pkg: preact Related to Preact (scope) labels Aug 2, 2022
@matthewp matthewp assigned matthewp and unassigned matthewp Aug 4, 2022
@bluwy
Copy link
Member

bluwy commented Aug 5, 2022

I'll take this too! Blocked by #4093

@bluwy
Copy link
Member

bluwy commented Aug 9, 2022

I believe #4213 should fix this too. I saw the same error while making the test and can confirm it's fixed with the PR.

@mayank99
Copy link
Contributor Author

it looks like this problem is still happening on astro@1.0.2 and @astrojs/preact@1.0.1 🙁 Getting the same error.

#4093 also seems to be still broken.

@bluwy
Copy link
Member

bluwy commented Aug 11, 2022

I was able to get dev working by specifying vite.ssr.noExternal: ['react-aria', 'react-stately', '@react-stately/*', '@react-aria/*']. Same for #4093 too with vite.ssr.noExternal: ['downshift']. The reason we need to do so is that in order for us to alias react > preact/compat, we need to process those libraries, instead of letting nodejs handle it.

I'd agree that it's a bit cumbersome, but I actually found a nicer workflow using this suggestion.

I'll re-open this PR, so we can suggest using npm:@preact/compat in the docs. That should make things configless, but for some reason it's failing the build, which I have to investigate.

@bluwy bluwy reopened this Aug 11, 2022
@bluwy
Copy link
Member

bluwy commented Aug 11, 2022

Well turns out the build fails because now preact suffers from dual package hazard. Astro calls preact-render-to-string's ESM code, which import preact in ESM, and some deps only export CJS, which requires preact in CJS. That's a bit annoying, and I'm not sure how this can be fixed. Either solutions so far isn't ideal for DX.

@bluwy
Copy link
Member

bluwy commented Aug 11, 2022

@mayank99 I added docs in #4267 which hopefully clarifies things a bit. From the repro here and at #4093. The build is still failing as:

  1. This repro: @react-aria/datepicker is incorrectly importing @internationailized/string when specifying vite.ssr.noExternal: ['react-aria', 'react-stately', '@react-stately/*', '@react-aria/*']
 error   Named export 'LocalizedStringDictionary' not found. The requested module '@internationalized/string' is a CommonJS module, which may not support all module.exports as named exports.
  CommonJS modules can always be imported via the default export, for example using:
  
  import pkg from '@internationalized/string';
  const { LocalizedStringFormatter, LocalizedStringDictionary } = pkg;
  1. #4093 repro: downshift is incorrectly exporting ESM. https://publint.bjornlu.com/downshift@6.1.9
  2. #4093 repro: When specifying vite.ssr.noExternal: ['downshift'], a bug in Vite occurs which I'll be fixing separately.
 error   Package subpath './helpers/esm/objectWithoutPropertiesLoose.js' is not defined by "exports" in /Users/bjorn/Downloads/github-bltmyz-5rgtvb/node_modules/@babel/runtime/package.json imported from /Users/bjorn/Downloads/github-bltmyz-5rgtvb/dist/entry.mjs

@mayank99
Copy link
Contributor Author

@bluwy Thank you so much for looking into this. Do you think from this point on, these issues should be reported to the respective packages? Since they seem to be using incorrect imports

@bluwy
Copy link
Member

bluwy commented Aug 11, 2022

Yeah the packages above are built for bundlers only, and not very friendly in nodejs, so it may be worth opening an issue on the respective packages. Those should be out of scope of Astro/Vite as it's a nodejs behaviour.

@igorbt
Copy link

igorbt commented Aug 15, 2022

I had the same issue trying to use Material UI (MUI) components. It seems like they have the same issue with incorrectly exporting ESM (https://publint.bjornlu.com/@mui/material@5.10.0) and have a Draft PR to fix this, but probably in the next major (mui/material-ui#30510).
A workaround that worked for me (I use pnpm) was to add this in package.json:

"pnpm": {
    "overrides": {
      "react": "npm:@preact/compat",
      "react-dom": "npm:@preact/compat"
    }
  }

and shamefully-hoist=true in .npmrc. I use pnpm workspaces so I did this at root level, not in the package with astro app. Hope it helps someone.

Later edit: Actually for some reason this workaround only works for astro dev. When running astro build it displays the true error: '...../@mui/material/Button' is not supported resolving ES modules. So while the ESM export is not fixed in MUI, need to use client:only🤦🏻.

@amirrezaDev1378
Copy link

hello, I am still experiencing this issue on Astro 4.2.8, is this an issue with preact-compat?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) pkg: preact Related to Preact (scope)
Projects
None yet
6 participants