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

fix: add support for node-like module resolution #4748

Merged
merged 5 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions .changeset/sour-plums-pull.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
"wrangler": patch
---

fix: resolve imports in a more node-like fashion for packages that do not declare exports

Previously, trying to import a file that wasn't explicitly exported from a module would result in an error, but now, better attempts are made to resolve the import using node's module resolution algorithm. It's now possible to do things like this:

```js
import JPEG_DEC_WASM from "@jsquash/jpeg/codec/dec/mozjpeg_dec.wasm";
```

This works even if the `mozjpeg_dec.wasm` file isn't explicitly exported from the `@jsquash/jpeg` module.
Cherry marked this conversation as resolved.
Show resolved Hide resolved

Fixes #4726
1 change: 1 addition & 0 deletions fixtures/import-wasm-example/README.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# import-wasm-example

`import-wasm-example` is a test fixture that imports a `wasm` file from `import-wasm-static`, testing npm module resolution with wrangler imports.
It also imports a file that isn't technically exported from `import-wasm-static`, testing more traditional node module resolution.
11 changes: 9 additions & 2 deletions fixtures/import-wasm-example/src/index.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
// this is from the `import-wasm-static` fixture defined above
// and setup inside package.json to mimic an npm package
import multiply from "import-wasm-static/multiply.wasm";
import otherMultiple from "import-wasm-static/wasm/not-exported.wasm";

export default {
async fetch(request) {
// just instantiate and return something
// we're really just testing the import at the top of this file
// we're really just testing the imports at the top of this file
const multiplyModule = await WebAssembly.instantiate(multiply);
return new Response(`${multiplyModule.exports.multiply(7, 3)}`);
const otherModule = await WebAssembly.instantiate(otherMultiple);

const results = [
multiplyModule.exports.multiply(7, 3),
otherModule.exports.multiply(7, 3),
];
return new Response(results.join(", "));
},
};
2 changes: 1 addition & 1 deletion fixtures/import-wasm-example/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ describe("wrangler correctly imports wasm files with npm resolution", () => {
it("responds", async ({ expect }) => {
const response = await fetch(`http://${ip}:${port}/`);
const text = await response.text();
expect(text).toBe("21");
expect(text).toBe("21, 21");
});
});
2 changes: 2 additions & 0 deletions fixtures/import-wasm-static/README.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# import-wasm-static

`import-wasm-static` is a fixture that simply exports a `wasm` file via `package.json` exports to be used and imported in other fixtures, to test npm module resolution.

It also provides a `not-exported.wasm` file that is not exported via `package.json` exports, to test node module resolution.
Binary file added fixtures/import-wasm-static/wasm/not-exported.wasm
Binary file not shown.
7 changes: 7 additions & 0 deletions fixtures/import-wasm-static/wasm/not-exported.wat
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
(module
(func $multiply (param $p1 i32) (param $p2 i32) (result i32)
local.get $p1
local.get $p2
i32.mul)
(export "multiply" (func $multiply))
)
2 changes: 2 additions & 0 deletions packages/wrangler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@
"miniflare": "workspace:*",
"nanoid": "^3.3.3",
"path-to-regexp": "^6.2.0",
"resolve": "^1.22.8",
"resolve.exports": "^2.0.2",
"selfsigned": "^2.0.1",
"source-map": "0.6.1",
Expand Down Expand Up @@ -141,6 +142,7 @@
"@types/minimatch": "^5.1.2",
"@types/prompts": "^2.0.14",
"@types/react": "^17.0.37",
"@types/resolve": "^1.20.6",
"@types/serve-static": "^1.13.10",
"@types/shell-quote": "^1.7.2",
"@types/signal-exit": "^3.0.1",
Expand Down
14 changes: 14 additions & 0 deletions packages/wrangler/src/deployment-bundle/module-collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { readdirSync } from "node:fs";
import { readFile } from "node:fs/promises";
import path from "node:path";
import globToRegExp from "glob-to-regexp";
import { sync as resolveSync } from "resolve";
import { exports as resolveExports } from "resolve.exports";
import { UserError } from "../errors";
import { logger } from "../logger";
Expand Down Expand Up @@ -316,6 +317,19 @@ export function createModuleCollector(props: {
}
}

// Next try to resolve using the node module resolution algorithm
try {
const resolved = resolveSync(args.path, {
basedir: args.resolveDir,
});
filePath = resolved;
} catch (e) {
// We tried, now it'll just fall-through to the previous behaviour
// and ENOENT if the absolute file path doesn't exist.
}

// Finally, load the file and hash it
// If we didn't do any smart resolution above, this will attempt to load as an absolute path
const fileContent = await readFile(filePath);
const fileHash = crypto
.createHash("sha1")
Expand Down
101 changes: 39 additions & 62 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.