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

CF Worker cannot use "nodejs_compat" with 1.18.0 (and previous versions) due to being polyfilled as unsupported #6715

Closed
1 task done
ivogt opened this issue Jun 28, 2023 · 9 comments

Comments

@ivogt
Copy link
Contributor

ivogt commented Jun 28, 2023

What version of Remix are you using?

1.18.0 (happens on all previous versions)

Are all your remix dependencies & dev-dependencies using the same version?

  • Yes

Steps to Reproduce

  1. npx create-remix@latest
  2. Choose Cloudflare Workers
  3. Enabled "node_compat" in wrangler toml
    add: compatibility_flags = [ "nodejs_compat" ]
  4. Add to server.ts
import { AsyncLocalStorage } from "node:async_hooks";
new AsyncLocalStorage();
  1. try nom run dev
  2. Error is displayed:
service core:user:remix-cloudflare-workers: Uncaught Error: Node.js async_hooks module is not supported by JSPM core outside of Node.js

Expected Behavior

Do not polyfill node:async_hooks for workers or allow config to exclude from being poly filled.

Actual Behavior

node:async_hooks gets polyfilled by node-modules-polyfills which changes the module to unimplemented stub.

@ivogt ivogt changed the title CF Worker cannot use "nodejs_compat" with 1.18 (and previous) due to being polyfilled as unsupported CF Worker cannot use "nodejs_compat" with 1.18.0 (and previous versions) due to being polyfilled as unsupported Jun 28, 2023
@ivogt
Copy link
Contributor Author

ivogt commented Jun 29, 2023

Hi @markdalgleish , sorry for tagging you but I saw you are dealing with polyfills and wanted to bring this issue to your attention. node_compat is unusable with workers currently.

Due to the eco-system I suspect that one way to resolve it is to allow us to selectively set what gets polyfilled for browser and server e.g. serverPolyfills:['crypto'] / serverPolyfill:false or similar. #6547 attempts to fix this but we still need an option to polyfill Crypto/Buffer and turning it all off brings different set of problems.

Thank you.

@markdalgleish
Copy link
Member

@ivogt Thanks for adding some more context! I've just opened a PR upstream to add support for this to the underlying esbuild polyfills plugin.

@evanderkoogh
Copy link

Am I correct in understanding that there still needs to be a small change made in Remix to read a module configuration item and pass that through to the polyfill library?

@markdalgleish
Copy link
Member

Yes that's right. I'm also thinking we might want to exclude async_hooks by default in Remix when building for non-Node environments since it'll only ever throw an error at runtime anyway.

@evanderkoogh
Copy link

Yeah.. might be worth taking a peek in the codebase if there are other modules that will just throw.

@markdalgleish
Copy link
Member

@ivogt I've opened a PR that should address this: #6814.

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-cdb1d32-20230713 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 1.19.0-pre.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 1.19.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants