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

React Carbon Components <CodeSnippet> Errors Out on Vite Production Build Only But Works on Dev #28

Closed
7 tasks done
mplatt212 opened this issue Nov 9, 2022 · 4 comments · Fixed by rollup/plugins#1350

Comments

@mplatt212
Copy link

Describe the bug

I am part of a team that has started work on a preexisting React/Vite project that uses Carbon Components. All is well until Docker serves Nginx with a production build and the component produces the following error:
image

This is even while just trying to render a simple string:
image

The strange thing is that this only seems to affect the component and none of the other Carbon Components I have on the page. I've made a simple reproduction in StackBlitz as closely as I could without running it through Docker and Nginx. Running a Vite Build and then Vite Preview sometimes it seems to work but other times is doesn't render to the screen at all but without any errors.

vite.config.ts
image

package.json
image
image
image
image

Reproduction

https://stackblitz.com/edit/vitejs-vite-cygruj?file=package.json,vite.config.ts,src%2Fmain.tsx,index.html,src%2FApp.tsx&terminal=dev

Steps to reproduce

vite build
vite preview

System Info

Windows 10

Used Package Manager

yarn

Logs

No response

Validations

@haoqunjiang haoqunjiang added the bug Something isn't working label Nov 18, 2022
@haoqunjiang
Copy link
Member

Root cause:
<CodeSnippet> uses use-resize-observer/polyfilled (version 6) which depends on resize-observer-polyfill: https://unpkg.com/browse/use-resize-observer@6.1.0/polyfilled.js
which returns the global ResizeObserver constructor when it's available: https://github.com/que-etc/resize-observer-polyfill/blob/4a148452494a155656e4d04b9732b5914896b2e0/src/index.js#L4
As it's a UMD module, it's transformed by @rollup/plugin-commonjs on building, which would add a getAugmentedNamespace() wrapper (https://github.com/rollup/plugins/blob/master/packages/commonjs/src/helpers.js#L37), which doesn't handle the new case.

Could be easily fixed in Rollup.

@haoqunjiang haoqunjiang removed the bug Something isn't working label Nov 19, 2022
haoqunjiang referenced this issue in haoqunjiang/plugins Nov 19, 2022
…t is a class

Fixes https://github.com/vitejs/vite/issues/10853

Since rollup@3c00191
the namespace became callable when requiring ESM with function default,
but it isn't newable, leading to errors when the function is actually a
class.
shellscape referenced this issue in rollup/plugins Nov 27, 2022
…1350)

* fix: correctly wraps the default export from a CommonJS module when it is a class

Fixes https://github.com/vitejs/vite/issues/10853

Since 3c00191
the namespace became callable when requiring ESM with function default,
but it isn't newable, leading to errors when the function is actually a
class.

* test: update snapshots as the helper code has changed
@patak-dev patak-dev transferred this issue from vitejs/vite Dec 3, 2022
@haoqunjiang
Copy link
Member

Fixed in @rollup/plugin-commonjs v23.0.3, which is included in vite v4.0.0-alpha.6.

@vctqs1
Copy link

vctqs1 commented Dec 9, 2022

Fixed in @rollup/plugin-commonjs v23.0.3, which is included in vite v4.0.0-alpha.6.

i'm using vite@v3, do we have an update in v3?

@haoqunjiang
Copy link
Member

Vite 4 contains very few breaking changes, so I recommend upgrading to it if possible.

If not, you can open a PR in the Vite repository to backport the dependency upgrade to the v3 branch.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants