-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
[base-ui] "useId" import error after updating esbuild #41190
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
Comments
I'm getting the same issue but with
|
@patrykszwed Could you also provide a minimal repro for the repo where you finally use this built package ? |
@brijeshb42 sure thing. I've updated the one linked in the description. It is enough to:
A warning about Logs
➜ final-app git:(main) npm run build
asset bundle.js 421 KiB [emitted] [minimized] [big] (name: main) 1 related asset WARNING in ../lib/esm/index.js 1864:24-48 WARNING in ../lib/esm/index.js 1864:51-75 WARNING in ../lib/esm/index.js 2431:22-34 WARNING in asset size limit: The following asset(s) exceed the recommended size limit (244 KiB). WARNING in entrypoint size limit: The following entrypoint(s) combined asset size exceeds the recommended limit (244 KiB). This can impact web performance. WARNING in webpack performance recommendations: webpack 5.90.3 compiled with 6 warnings in 8831 ms Here's a link to the repo itself - mui-repro-lib. Let me know in case you need anything else and thanks for looking into this. |
Thanks. I will investigate. |
@brijeshb42 sure, I've added and setup a |
ReproductionI can reproduce on https://esbuild.github.io/try/#dAAwLjE5LjYALS1taW5pZnkAcmV0dXJuICdzZWYnLnRvU3RyaW5nKCk7 Screen.Recording.2024-02-25.at.16.15.33.movThis is since esbuild v0.19.6 (Nov 19, 2023). ContextSo If I understand correctly:
![]()
![]() Options
cc @atomiks too as https://github.com/floating-ui/floating-ui/blob/495954da10d2abe8a5e6a0f0247582f071d6f031/packages/react/src/hooks/useId.ts#L30 is a dependency of ![]() https://npm.anvaka.com/#/view/2d/%2540mui%252Fbase ![]() @brijeshb42 The @mui/utils package is mostly an npm dependency of Base UI, I fixed the labels (a bit MUI System too but it's rarer). I fixed this. |
So simply passing an argument like Here's how Ariakit does it: import * as React from "react";
// See https://github.com/webpack/webpack/issues/14814
const _React = { ...React };
const useReactId = _React.useId; |
I've also verified that |
@patrykszwed Just curious, why not keep |
@brijeshb42 our setup is pretty wild and we're using Juggling between different webpack and rollup configurations results, in most cases, in either:
|
Hey @brijeshb42, I just wanted to ask if there's a plan to fix that issue in the nearest future? I took a look into this PR in the esbuild v0.19.6: esbuild v0.20.2: |
I think the goal is to make it impossible to statically analyze. I saw a screenshot where React used React[`useId${Math.random()}`.slice(0, 5)] The other technique that might be future proof is: const _React = {...React};
const useReactId = _React.useId; |
Use technique from this comment: mui/material-ui#41190 (comment) to further obfuscate reference to `React.useId` which causes issue when using `react-resizable-panels` with React <18 and `esbuild` > 0.19.5. Fixes bvaughn#381
Use technique from this comment: mui/material-ui#41190 (comment) to further obfuscate reference to `React.useId` which causes issue when using `react-resizable-panels` with React <18 and `esbuild` > 0.19.5. Fixes #381 --------- Co-authored-by: Brian Vaughn <[email protected]>
Hello team! We are also bundling MUI. However we are facing the issue for
I like the idea proposed by @atomiks
@oliviertassinari and @brijeshb42, I would love to contribute to this. If you guys agree with atomiks's proposal then I can raise a PR. |
I agree, we should fix this in Material UI too 👍. On the options proposed here, I can barely find any differences between: Option A React[`useId${Math.random()}`.slice(0, 5)] and, Option B const _React = {...React};
const useReactId = _React.useId; The runtime cost is almost the same, A: 0.005126953125 ms vs. B: 0.010986328125 ms. React could have tree-shakeable exports in the future facebook/react#11503, but both option looks equally good in that context. Option B might compress a bit better with brottli/gzip since it has more common constructs, e.g. Going with what @atomiks used for Floating UI, since Base UI uses @mui/utils/useId, seems best, we will be consistent this way, so Option B. |
@oliviertassinari I've raised the PR #43360 Please let me know if I've missed anything. |
This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue. Note We value your feedback @patrykszwed! How was your experience with our support team? |
Steps to reproduce
Link to live example: CodeSandbox
Steps:
npm run build
./lib/cjs/index.csj.js
and/lib/esm/index.js
files.maybeReactUseId
variable in both of them.Current behavior
Note: this issue is similar to #37182, #37183 and #29860. Its root cause is probably related to that Webpack issue.
Observations
What can be observed in
/lib/cjs/index.cjs.js
file:What can be observed in
/lib/esm/index.js
file:That difference is caused by enabling
minifySyntax: true
inbundle.js
only for the "esm" format.esbuild
was not minifying["useId".toString()]
to.useId
until version0.18.20
(including). However, it's been doing this since0.19.0
release (I've checked the latestesbuild
version and the outcome is the same).Scenario
Now let's say this package is an internal components library which is reused in another app that also uses React 17. When one tries to build that app using
webpack
an error will be thrown:I've encountered this issue after updating Vite, in that internal components library, from version 4.4.12 to 5.1.3. Vite uses
esbuild
for client builds minification by default, and in 5.0 they bumped its requiredesbuild
version from^0.18.0
to^0.19.0
. This is how theuseId
hook is called in the output code in that component library:As a result, when I'm trying to build my main app (that uses the components library) using
webpack
, the build fails.Expected behavior
Bundled
maybeReactUseId
variable (or any other mangled name), should probably use a bracket notation even whenminifySyntax
is enabled on[email protected]
(or newer).Context
I have two packages:
17.0.2
5.15.10
5.1.3
0.19.12
17.0.2
5.90.3
When I'm trying to build the main app using
webpack
, I'm getting the error described above. It is possible to work around that by settingminifySyntax: false
inesbuild
settings but it'd be great that wouldn't turn out to be the only way.Your environment
A browser doesn't matter in that case.npx @mui/envinfo
Search keywords: esbuild useId utils webpack vite react
The text was updated successfully, but these errors were encountered: