Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add Node.js Compat module type #564

Merged
merged 2 commits into from
Apr 28, 2023
Merged

Add Node.js Compat module type #564

merged 2 commits into from
Apr 28, 2023

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Apr 24, 2023

Just an initial proof-of-concept on the idea of a Node.js module type.

This type of module that can be included in the worker bundle allows for requiring node.js built-ins without the node: specifier-prefix and would expose the common Node.js globals

There is some complexity here that would still need to be worked through. This will remain a draft PR until we determine if this is the way we'd want to go.

src/workerd/jsg/modules.h Outdated Show resolved Hide resolved
src/workerd/jsg/modules.h Outdated Show resolved Hide resolved
Copy link
Collaborator

@irvinebroque irvinebroque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together, helpful starting point for discussion. Notes below:

How would one declare a dependency a nodeJsModule type?

Am inferring a bit — is the idea here that Wrangler (or another bundler) would, as part of bundling:

  1. Identify "this looks like a Node.js dependency" (ex: require())
  2. Generate a separate bundle for these dependencies (one for all of them or one for each?)
  3. Declare somehow that this bundle is of type nodeJsModule

We can talk internally about this piece as there are moving pieces beyond workerd, but I think spelling out how this would work to application developers, and working backwards from there, would be helpful.

Shipping incrementally in pieces

I wonder if we could land this incrementally, and break down the problem into separate pieces:

  1. global require() is available
  2. require("buffer") works (such that a dependency does not need to use node: prefix
  3. globals are available as they are in Node.js

It seems like (1) and (2) are much more important than (3) — more stuff still not using ES modules & requiring node dependencies without node:, less stuff relying on magic globals. And (3) is where the complexity is, and where some of the open questions are.

@jasnell
Copy link
Member Author

jasnell commented Apr 25, 2023

How would one declare a dependency a nodeJsModule type?
// ...
Declare somehow that this bundle is of type nodeJsModule
Identify "this looks like a Node.js dependency" (ex: require())
Generate a separate bundle for these dependencies (one for all of them or one for each?)

Not sure what you mean. Workers already supports multiple module types (esModule, commonJsModule, json, etc). This becomes just another one of those. Wrangler will need to be updated to figure out when it use it but "NodeJS" should become just another key in the type field in the wrangler.toml (https://developers.cloudflare.com/workers/wrangler/configuration/#bundling)

  1. global require() is available
  2. require("buffer") works (such that a dependency does not need to use node: prefix
  3. globals are available as they are in Node.js
    It seems like (1) and (2) are much more important than (3)

Unfortunately 3 is just as important. Use of common Node.js globals like Buffer and process are just as ubiquitous as require(...).

Note that the complexity around making the globals available is only about the logistics of doing so, not about whether we should or not. There's some complexity here regarding the layering between workerd::api and workerd:jsg that we'll need to work through.

@jasnell jasnell force-pushed the jsnell/add-nodejs-module-type branch from 9398d12 to 1138b55 Compare April 25, 2023 19:42
@jasnell jasnell marked this pull request as ready for review April 25, 2023 19:46
@jasnell jasnell changed the title [POC] Add basic scaffolding for Node.js module type Add basic scaffolding for Node.js module type Apr 25, 2023
@jasnell jasnell force-pushed the jsnell/add-nodejs-module-type branch from 1138b55 to 5452abd Compare April 25, 2023 19:53
@jasnell jasnell changed the title Add basic scaffolding for Node.js module type Add Node.js Compat module type Apr 25, 2023
samples/nodejs-compat-module/foo.js Show resolved Hide resolved
src/workerd/api/node/context.c++ Outdated Show resolved Hide resolved
@jasnell jasnell force-pushed the jsnell/add-nodejs-module-type branch 5 times, most recently from 214cebe to 1233c32 Compare April 28, 2023 15:23
@jasnell jasnell force-pushed the jsnell/add-nodejs-module-type branch from 1233c32 to 2c0c0d0 Compare April 28, 2023 15:23
@jasnell jasnell merged commit 23dd3ac into main Apr 28, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants