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

feat: support npm resolution for file imports #4135

Merged
merged 15 commits into from Oct 24, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 19 additions & 0 deletions .changeset/long-starfishes-mate.md
@@ -0,0 +1,19 @@
---
"wrangler": minor
---

feat: resolve npm exports for file imports

Previously, when using wasm (or other static files) from an npm package, you would have to import the file like so:

```js
import wasm from "../../node_modules/svg2png-wasm/svg2png_wasm_bg.wasm";
```

This update now allows you to import the file like so, assuming it's exposed and available in the package's `exports` field:

```js
import wasm from "svg2png-wasm/svg2png_wasm_bg.wasm";
```

This will look at the package's `exports` field in `package.json` and resolve the file using [`resolve.exports`](https://www.npmjs.com/package/resolve.exports).
1 change: 1 addition & 0 deletions fixtures/import-wasm-example/.gitignore
@@ -0,0 +1 @@
dist
3 changes: 3 additions & 0 deletions fixtures/import-wasm-example/README.md
@@ -0,0 +1,3 @@
# 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.
23 changes: 23 additions & 0 deletions fixtures/import-wasm-example/package.json
@@ -0,0 +1,23 @@
{
"name": "import-wasm-example",
"version": "1.0.1",
"private": true,
"description": "",
"author": "",
"main": "src/index.js",
"scripts": {
"check:type": "tsc",
"test": "npx vitest run",
"test:ci": "npx vitest run",
"test:watch": "npx vitest",
"type:tests": "tsc -p ./tests/tsconfig.json"
},
"devDependencies": {
"undici": "^5.9.1",
"wrangler": "workspace:*",
"@cloudflare/workers-tsconfig": "workspace:^"
},
"dependencies": {
"import-wasm-static": "workspace:^"
}
}
12 changes: 12 additions & 0 deletions fixtures/import-wasm-example/src/index.js
@@ -0,0 +1,12 @@
// 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";

export default {
async fetch(request) {
// just instantiate and return something
// we're really just testing the import at the top of this file
const multiplyModule = await WebAssembly.instantiate(multiply);
return new Response(`${multiplyModule.exports.multiply(7, 3)}`);
},
};
25 changes: 25 additions & 0 deletions fixtures/import-wasm-example/tests/index.test.ts
@@ -0,0 +1,25 @@
import { resolve } from "path";
import { fetch } from "undici";
import { describe, it, beforeAll, afterAll } from "vitest";
import { runWranglerDev } from "../../shared/src/run-wrangler-long-lived";

describe("wrangler correctly imports wasm files with npm resolution", () => {
let ip: string, port: number, stop: (() => Promise<unknown>) | undefined;

beforeAll(async () => {
({ ip, port, stop } = await runWranglerDev(resolve(__dirname, ".."), [
"--port=0",
]));
});

afterAll(async () => {
await stop?.();
});

// if the worker compiles, is running, and returns 21 (7 * 3) we can assume that the wasm module was imported correctly
it("responds", async ({ expect }) => {
const response = await fetch(`http://${ip}:${port}/`);
const text = await response.text();
expect(text).toBe("21");
});
});
7 changes: 7 additions & 0 deletions fixtures/import-wasm-example/tests/tsconfig.json
@@ -0,0 +1,7 @@
{
"extends": "@cloudflare/workers-tsconfig/tsconfig.json",
"compilerOptions": {
"types": ["node"]
},
"include": ["**/*.ts", "../../../node-types.d.ts"]
}
12 changes: 12 additions & 0 deletions fixtures/import-wasm-example/tsconfig.json
@@ -0,0 +1,12 @@
{
"compilerOptions": {
"target": "ES2020",
"esModuleInterop": true,
"module": "CommonJS",
"lib": ["ES2020"],
"types": ["node"],
"moduleResolution": "node",
"noEmit": true
},
"include": ["tests", "../../node-types.d.ts"]
}
4 changes: 4 additions & 0 deletions fixtures/import-wasm-example/wrangler.toml
@@ -0,0 +1,4 @@
name = "import-wasm-example"
compatibility_date = "2023-10-02"

main = "src/index.js"
3 changes: 3 additions & 0 deletions fixtures/import-wasm-static/README.md
@@ -0,0 +1,3 @@
# 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.
9 changes: 9 additions & 0 deletions fixtures/import-wasm-static/package.json
@@ -0,0 +1,9 @@
{
"name": "import-wasm-static",
"version": "0.0.1",
"private": true,
"sideEffects": false,
"exports": {
"./multiply.wasm": "./wasm/multiply.wasm"
}
}
Binary file added fixtures/import-wasm-static/wasm/multiply.wasm
Binary file not shown.
7 changes: 7 additions & 0 deletions fixtures/import-wasm-static/wasm/multiply.wat
@@ -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))
)
1 change: 1 addition & 0 deletions packages/wrangler/package.json
Expand Up @@ -110,6 +110,7 @@
"miniflare": "3.20231016.0",
"nanoid": "^3.3.3",
"path-to-regexp": "^6.2.0",
"resolve.exports": "^2.0.2",
"selfsigned": "^2.0.1",
"source-map": "0.6.1",
"source-map-support": "0.5.21",
Expand Down
20 changes: 20 additions & 0 deletions packages/wrangler/src/__tests__/module-collection.test.ts
@@ -0,0 +1,20 @@
import { extractPackageName } from "../deployment-bundle/module-collection";

describe("Module Collection", () => {
describe("extractPackageName", () => {
test.each`
importString | packageName
${"wrangler"} | ${"wrangler"}
${"wrangler/example"} | ${"wrangler"}
${"wrangler/example.wasm"} | ${"wrangler"}
${"@cloudflare/wrangler"} | ${"@cloudflare/wrangler"}
${"@cloudflare/wrangler/example"} | ${"@cloudflare/wrangler"}
${"@cloudflare/wrangler/example.wasm"} | ${"@cloudflare/wrangler"}
${"./some/file"} | ${null}
${"../some/file"} | ${null}
${"/some/file"} | ${null}
`("$importString --> $packageName", ({ importString, packageName }) => {
expect(extractPackageName(importString)).toBe(packageName);
});
});
});
5 changes: 4 additions & 1 deletion packages/wrangler/src/deployment-bundle/bundle.ts
Expand Up @@ -31,6 +31,9 @@ export const COMMON_ESBUILD_OPTIONS = {
loader: { ".js": "jsx", ".mjs": "jsx", ".cjs": "jsx" },
} as const;

// build conditions used by esbuild, and when resolving custom `import` calls
export const BUILD_CONDITIONS = ["workerd", "worker", "browser"];

/**
* Information about Wrangler's bundling process that needs passed through
* for DevTools sourcemap transformation
Expand Down Expand Up @@ -310,7 +313,7 @@ export async function bundleWorker(
sourceRoot: destination,
minify,
metafile: true,
conditions: ["workerd", "worker", "browser"],
conditions: BUILD_CONDITIONS,
...(process.env.NODE_ENV && {
define: {
// use process.env["NODE_ENV" + ""] so that esbuild doesn't replace it
Expand Down
66 changes: 65 additions & 1 deletion packages/wrangler/src/deployment-bundle/module-collection.ts
Expand Up @@ -3,7 +3,9 @@
import { readFile } from "node:fs/promises";
import path from "node:path";
import globToRegExp from "glob-to-regexp";
import { exports as resolveExports } from "resolve.exports";
import { logger } from "../logger";
import { BUILD_CONDITIONS } from "./bundle";
import {
findAdditionalModules,
findAdditionalModuleWatchDirs,
Expand Down Expand Up @@ -64,6 +66,23 @@
},
};

// Extracts a package name from a string that may be a file path
// or a package name. Returns null if the string is not a valid
// Handles `wrangler`, `wrangler/example`, `wrangler/example.wasm`,
// `@cloudflare/wrangler`, `@cloudflare/wrangler/example`, etc.
export function extractPackageName(packagePath: string) {
if (packagePath.startsWith(".")) return null;

const match = packagePath.match(/^(@[^/]+\/)?([^/]+)/);
Cherry marked this conversation as resolved.
Show resolved Hide resolved

if (match) {
const scoped = match[1] || "";
const packageName = match[2];
return `${scoped}${packageName}`;
}
return null;
}

export function createModuleCollector(props: {
entry: Entry;
findAdditionalModules: boolean;
Expand Down Expand Up @@ -237,7 +256,7 @@
// take the file and massage it to a
// transportable/manageable format

const filePath = path.join(args.resolveDir, args.path);
let filePath = path.join(args.resolveDir, args.path);

// If this was a found additional module, mark it as external.
// Note, there's no need to watch the file here as we already
Expand All @@ -251,6 +270,51 @@
// it to `esbuild` to bundle it.
if (isJavaScriptModuleRule(rule)) return;

// Check if this file is possibly from an npm package
// and if so, validate the import against the package.json exports
// and resolve the file path to the correct file.
if (args.path.includes("/") && !args.path.startsWith(".")) {
// get npm package name from string, taking into account scoped packages
const packageName = extractPackageName(args.path);

Check warning on line 278 in packages/wrangler/src/deployment-bundle/module-collection.ts

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/deployment-bundle/module-collection.ts#L278

Added line #L278 was not covered by tests
if (!packageName) {
throw new Error(

Check warning on line 280 in packages/wrangler/src/deployment-bundle/module-collection.ts

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/deployment-bundle/module-collection.ts#L280

Added line #L280 was not covered by tests
`Unable to extract npm package name from ${args.path}`
);
}
const packageJsonPath = path.join(

Check warning on line 284 in packages/wrangler/src/deployment-bundle/module-collection.ts

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/deployment-bundle/module-collection.ts#L284

Added line #L284 was not covered by tests
process.cwd(),
"node_modules",
packageName,
"package.json"
);
// Try and read the npm package's package.json
// and then resolve the import against the package's exports
// and then finally override filePath if we find a match.
try {
const packageJson = JSON.parse(

Check warning on line 294 in packages/wrangler/src/deployment-bundle/module-collection.ts

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/deployment-bundle/module-collection.ts#L293-L294

Added lines #L293 - L294 were not covered by tests
await readFile(packageJsonPath, "utf8")
);
const testResolved = resolveExports(

Check warning on line 297 in packages/wrangler/src/deployment-bundle/module-collection.ts

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/deployment-bundle/module-collection.ts#L297

Added line #L297 was not covered by tests
packageJson,
args.path.replace(`${packageName}/`, ""),
{
conditions: BUILD_CONDITIONS,
}
);
if (testResolved) {
filePath = path.join(

Check warning on line 305 in packages/wrangler/src/deployment-bundle/module-collection.ts

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/deployment-bundle/module-collection.ts#L305

Added line #L305 was not covered by tests
process.cwd(),
"node_modules",
packageName,
testResolved[0]
);
}
} catch (e) {
// We tried, now it'll just fall-through to the previous behaviour
// and ENOENT if the absolute file path doesn't exist.
}
}

const fileContent = await readFile(filePath);
const fileHash = crypto
.createHash("sha1")
Expand Down