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.

Sign up for GitHub

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

Jump to bottom

fix: avoid esbuild warning when running dev/bundle #5999

Closed
wants to merge 1 commit into from
Closed

fix: avoid esbuild warning when running dev/bundle #5999

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .changeset/famous-chicken-glow.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
"wrangler": patch
---

fix: avoid esbuild warning when running dev/bundle

I've been experimenting with esbuild 0.21.4 with wrangler. It's mostly been fine. But I get this warning every time

```
▲ [WARNING] Import "__INJECT_FOR_TESTING_WRANGLER_MIDDLEWARE__" will always be undefined because there is no matching export in "src/index.ts" [import-is-undefined]

.wrangler/tmp/bundle-Z3YXTd/middleware-insertion-facade.js:8:23:
8 │ .....(OTHER_EXPORTS.__INJECT_FOR_TESTING_WRANGLER_MIDDLEWARE__ ?? []),
╵
```

This is because [email protected] enabled a warning by default whenever an undefined import is accessed on an imports object. However we abuse imports to inject stuff in `middleware.test.ts`. We should probably fix that with a better solution, but an immediate fix to this warning is to make that access not be statically analyzable. This patch does so by defining the export name as a regular variable and using that when reading from it.
17 changes: 15 additions & 2 deletions packages/wrangler/src/__tests__/middleware.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -861,7 +861,19 @@ describe("middleware", () => {
.replace(/\/\/ .*/g, "")
.trim()
).toMatchInlineSnapshot(`
"var src_default = {
"var __defProp = Object.defineProperty;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

of note, it adds some extra code for reading that dynamically. which is why we need a proper fix later so we don't ship any extra code. but this should be fine for now.

var __export = (target, all) => {
for (var name in all)
__defProp(target, name, { get: all[name], enumerable: true });
};


var src_exports = {};
__export(src_exports, {
DurableObjectExample: () => DurableObjectExample,
default: () => src_default
});
var src_default = {
async fetch(request, env) {
return Response.json(env);
}
Expand All @@ -875,8 +887,9 @@ describe("middleware", () => {
};


var testingMiddlewareExportName = \\"__INJECT_FOR_TESTING_WRANGLER_MIDDLEWARE__\\";
var __INTERNAL_WRANGLER_MIDDLEWARE__ = [
...void 0 ?? []
...src_exports[testingMiddlewareExportName] ?? []
];
var middleware_insertion_facade_default = src_default;

Expand Down
4 changes: 3 additions & 1 deletion packages/wrangler/src/deployment-bundle/apply-middleware.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,10 @@ export async function applyMiddlewareLoaderFacade(

export * from "${prepareFilePath(entry.file)}";

const testingMiddlewareExportName = "__INJECT_FOR_TESTING_WRANGLER_MIDDLEWARE__";

export const __INTERNAL_WRANGLER_MIDDLEWARE__ = [
...(OTHER_EXPORTS.__INJECT_FOR_TESTING_WRANGLER_MIDDLEWARE__ ?? []),
...(OTHER_EXPORTS[testingMiddlewareExportName] ?? []),
${middlewareFns}
]
export default worker;
Expand Down
Loading