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

Leverage nodejs conditional exports #35

Closed
blaine-arcjet opened this issue Oct 23, 2023 · 5 comments · Fixed by #887
Closed

Leverage nodejs conditional exports #35

blaine-arcjet opened this issue Oct 23, 2023 · 5 comments · Fixed by #887
Assignees
@blaine-arcjet

Comments

@blaine-arcjet
Copy link
Contributor

I think we can leverage nodejs conditional exports to adjust our Wasm imports for the platform we are targeting.

Depending on how tools handle packages, this should reduce some false positives for any capabilities checks, such as if you are using arcjet-analyze on the edge or with a bundler and don't need the filesystem read we provide for nodejs.

We could use "worker", "browser", and "default" keys.

See also vercel/next.js#39094
@blaine-arcjet blaine-arcjet self-assigned this Oct 23, 2023
@blaine-arcjet
Copy link
Contributor Author

I also want to investigate if this will allow us to avoid dynamic imports and the need to set the asyncWebAssembly in webpack

@blaine-arcjet
Copy link
Contributor Author

While digging into next.js, I found that they use quite an eclectic series of fields and it varies based on build target (edge, node/SSR, and client). It'll take some time to figure out what works where.

@blaine-arcjet blaine-arcjet transferred this issue from another repository Dec 13, 2023
@blaine-arcjet blaine-arcjet removed their assignment Apr 16, 2024
@blaine-arcjet blaine-arcjet self-assigned this Jun 6, 2024
@blaine-arcjet
Copy link
Contributor Author

blaine-arcjet commented Jun 6, 2024 •
edited
Loading

I found that we can use "edge-light" for Vercel: https://github.com/vercel/next.js/blob/04ce445105eed981f0d137888fc977f1b3df1d50/packages/next/src/build/webpack-config-rules/resolve.ts#L8

@blaine-arcjet
Copy link
Contributor Author

Cloudflare seems to prefer "workerd" over "worker" now: https://github.com/cloudflare/workers-sdk/blob/1e68fe5448ffa4d0551dc7255405983c329235c8/packages/wrangler/src/deployment-bundle/bundle.ts#L37

@blaine-arcjet blaine-arcjet mentioned this issue Jun 6, 2024
Merged
@blaine-arcjet
Copy link
Contributor Author

See also: cloudflare/workers-sdk#2629

@trunk-io trunk-io bot closed this as completed in #887 Jun 7, 2024
trunk-io bot pushed a commit that referenced this issue Jun 7, 2024
@blaine-arcjet
chore(analyze)!: Leverage conditional exports to load Wasm appropriat…
d7a698f 
…ely (#887)

Closes #35 

This leverages conditional exports for various platforms: `"edge-light"` for Vercel, `"workerd"` for Cloudflare, and `"default"` for our base64 Wasm implementation.

There is a lot of code duplication because it's hard to reason about the exports from inside the package and I didn't want to try to design an API that would be imported into each file.

I also had to rework our Rollup logic because the Cloudflare docs don't match their implementation as of the newest Wrangler.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@blaine-arcjet blaine-arcjet

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore(analyze)!: Leverage conditional exports to load Wasm appropriately arcjet/arcjet-js
1 participant
@blaine-arcjet