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

[🐛 Bug]: Not all workers-compatible nodejs api is fully supported #249

Closed
dario-piotrowicz opened this issue May 15, 2023 · 2 comments · Fixed by #268
Closed

[🐛 Bug]: Not all workers-compatible nodejs api is fully supported #249

dario-piotrowicz opened this issue May 15, 2023 · 2 comments · Fixed by #268
Assignees
Labels
bug Something isn't working in progress This issue is currently being worked on and show be solved soon.

Comments

@dario-piotrowicz
Copy link
Member

dario-piotrowicz commented May 15, 2023

Description

Not all nodejs API supported by the workers runtime (docs) is correctly supported, of the compatibility list:

I think that we only support AsyncLocalStorage and Buffer but not the rest.

Support should be added since we already require the nodejs_compat flag to be specified so we should actually make sure that we support all the APIs provided by the flag.


Reproduction

For example see the api/hello route of this app: simple-pages-dir-13.4.2.

It generated an internal server error because of the use of node:events:
Screenshot 2023-05-16 at 00 06 01

(Note that EventEmitters are workers-compatible as indicated above)

@dario-piotrowicz dario-piotrowicz added the bug Something isn't working label May 15, 2023
@dario-piotrowicz dario-piotrowicz self-assigned this May 15, 2023
@dario-piotrowicz
Copy link
Member Author

Related to: #244


@felix-schultz I'm pinging you in case you we'd be interested in following this 🙂, it does look like we can do things better in regards of node compatibility, I'll look into this soon 🙂👍

@dario-piotrowicz dario-piotrowicz added the blocked by external This can't be currently solved because of external factors (Vercel/Next, Pages, etc...) label May 16, 2023
@dario-piotrowicz
Copy link
Member Author

Note (to self)

currently we have this ugly hack that is needed because of a workers runtime bug:

// TODO: remove ASAP (after runtime fix) @dario
await mkdir(nextOnPagesDistDir, { recursive: true });
await writeFile(
join(nextOnPagesDistDir, '..', 'node-buffer.js'),
'export * from "node:buffer"'
);
///////////////////////////////////////////////

this would make the solution for nodejs modules require such hacks for all modules potentially making a bit of a mess, since I don't believe that there is a high demand for this fix (and the runtime bug should be fixed fairly soon) let's wait for the runtime to be fixed first (and the above hack removed) before attempting to support all node.js modules.

@dario-piotrowicz dario-piotrowicz added in progress This issue is currently being worked on and show be solved soon. and removed blocked by external This can't be currently solved because of external factors (Vercel/Next, Pages, etc...) labels May 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working in progress This issue is currently being worked on and show be solved soon.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant