fix: silent IMPORT_IS_UNDEFINED for proxy.ts#787
fix: silent IMPORT_IS_UNDEFINED for proxy.ts#787hyoban wants to merge 1 commit intocloudflare:mainfrom
Conversation
commit: |
|
/bigbonk review |
There was a problem hiding this comment.
Good fix for a real annoyance. The approach is correct — suppressing bundler noise for known-safe generated patterns. Two issues worth addressing:
-
Missing
.tsx/.jsxextensions —findMiddlewareFile()iterates allpageExtensions(default:tsx, ts, jsx, js), so a user withproxy.tsxormiddleware.jsxwould still see the warning. The filter should cover all possible extensions, or match more broadly. -
Named export warnings not covered — The generated entry code does
middlewareModule.proxy ?? middlewareModule.default(or.middleware ?? .default). When a file only exportsdefault, the bundler can also warn aboutImport \proxy` will always be undefined(and vice versa). The current filter only matchesImport `default``, missing the symmetric case.
| (warning.message?.includes("proxy.ts") || | ||
| warning.message?.includes("proxy.js") || | ||
| warning.message?.includes("middleware.ts") || | ||
| warning.message?.includes("middleware.js")) && |
There was a problem hiding this comment.
This only covers .ts and .js, but findMiddlewareFile() iterates all fileMatcher.dottedExtensions which defaults to [".tsx", ".ts", ".jsx", ".js"] (and can include custom extensions via pageExtensions config). A user with proxy.tsx or middleware.jsx would still see the warning.
Consider matching on just the basename without extension, e.g.:
| warning.message?.includes("middleware.js")) && | |
| if ( | |
| warning.code === "IMPORT_IS_UNDEFINED" && | |
| (warning.message?.includes("virtual:vinext-rsc-entry") || | |
| warning.message?.includes("virtual:vinext-server-entry")) && | |
| /Import `(?:default|proxy|middleware)` will always be undefined/.test( | |
| warning.message ?? "", | |
| ) && | |
| /\b(?:proxy|middleware)\.\w+/.test(warning.message ?? "") | |
| ) { |
This also covers the symmetric case where the named export (proxy or middleware) triggers the warning instead of default (e.g., when the file only has a default export, middlewareModule.proxy is undefined).
| { | ||
| code: "IMPORT_IS_UNDEFINED", | ||
| message: | ||
| "[IMPORT_IS_UNDEFINED] Warning: Import `default` will always be undefined because there is no matching export in 'proxy.ts'\\n ╭─[ \\0virtual:vinext-rsc-entry:2632:34 ]", |
There was a problem hiding this comment.
The test message strings use \\n (literal backslash-n) instead of \n (actual newline). In real Rolldown output the message would contain an actual newline character. This doesn't affect correctness since .includes() would match either way in this test, but it's slightly misleading.
Also, consider adding test cases for:
.tsx/.jsxextensions (proxy.tsx,middleware.jsx)- Named export warnings (
Import \proxy` will always be undefined`) - The symmetric case where only a default export exists
|
Review posted successfully. Here's a summary of the feedback: PR #787 suppresses noisy
|
Uh oh!
There was an error while loading. Please reload this page.