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

Added no_nodejs_compat_v2 to notes #18765

Open
wants to merge 1 commit into
base: production
Choose a base branch
from

Conversation

ToriLindsay
Copy link
Contributor

Fixes #17608

Copy link

Deploying cloudflare-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 718459d
Status: ✅  Deploy successful!
Preview URL: https://1ecd2d7f.cloudflare-docs-7ou.pages.dev
Branch Preview URL: https://tori-pcx17608-add-no-nodejs.cloudflare-docs-7ou.pages.dev

View logs

Copy link
Member

@GregBrimble GregBrimble left a comment

Choose a reason for hiding this comment

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

I'd also like to see an actual Compatibility Flag entry for this in the list below. You can create one by creating a file like ./src/content/compatibility-flags/nodejs-compat-v2.md:

---
_build:
  publishResources: false
  render: never
  list: never

name: "Node.js compatibility v2"
sort_date: "2024-09-23"
enable_date: "2024-09-23"
enable_flag: "nodejs_compat_v2"
disable_flag: "no_nodejs_compat_v2"
---

This flag allows you to import Node.js modules without the `node:` prefix and will polyfill additional Node.js modules and globals that are not available with `nodejs_compat` alone.

@@ -50,7 +50,7 @@ You can opt into improved Node.js compatibility by using `nodejs_compat_v2` inst
additionally you can import Node.js modules without the `node:` prefix and use polyfilled Node.js modules and globals that are not available with `nodejs_compat`.

On September 23, 2024, `nodejs_compat` will use the improved Node.js compatibility currently enabled with `nodejs_compat_v2`. This will require updating your
compatibility_date to 2024-09-23 or later.
compatibility_date to 2024-09-23 or later. If you want to explicitly disable the Node.js v2 compatibility features to reduce bundle size, you can use the `no_nodejs_compat_v2` flag.
Copy link
Member

Choose a reason for hiding this comment

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

Take it or leave it:

Suggested change
compatibility_date to 2024-09-23 or later. If you want to explicitly disable the Node.js v2 compatibility features to reduce bundle size, you can use the `no_nodejs_compat_v2` flag.
compatibility_date to 2024-09-23 or later. If you want to explicitly disable these improved Node.js compatibility features (for example, to reduce bundle size), you can use the `no_nodejs_compat_v2` flag.

Copy link
Contributor

Choose a reason for hiding this comment

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

to reduce bundle size

Maybe we should explicitly explain the difference between nodejs_compat and nodejs_compat_v2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vicb @GregBrimble
I feel like, ideally, we’d just remove “to reduce bundle size” and then link “improved Node.js compatibility” to a an explanation in this article of the differences/improvements of v2. Could one of you either draft those v2 improvements here or in another PR or just list them for me here and I’ll add it? Thank you!
(But I also saw your other comments and I will make a task to take a more holistic look at improving the nodejs compat info, in general)
cc: @mikenomitch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(For the purposes of people who might want to use the no_nodejs_compat_v2 flag, we should also make any differences clear that could be disadvantages in some situations)

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like, ideally, we’d just remove “to reduce bundle size” and then link “improved Node.js compatibility” to a an explanation in this article of the differences/improvements of v2

+1 to remove details in most places where we and mention the compat flags and link to "this article".

(However I think that the text we are commenting on is located in the article).

About the differences:

v2 improves Node compatibility by using unenv. unenv implements some node modules that are not implemented by workerd. We have to bundle those implementations alongside the user code which explain why the bundle siize increases. But workerd is slowly catching up by implementing modules so that the unenv overhead is less and less significant. On top of that we have increased the allowed bundle size. Another reason for which bundle size is becoming less of a concern.

IMO the other differences "the globals Buffer and process are available everywhere" deserve to be describe. The only reason why I have seen no_nodejs_compat_v2 is to disable process update. @petebacondarwin has more details.

@anonrig Are there any other differences backed in nodejs_compat_v2?

@@ -32,7 +32,7 @@ compatibility_date = "2024-09-23"
</WranglerConfig>

:::note
As of September 23rd, 2024, the `nodejs_compat` compatibility flag enables the exact same behavior as the `nodejs_compat_v2` compatibility flag, as long as your compatibility date is set to September 23rd, 2024 or later.
As of September 23rd, 2024, the `nodejs_compat` compatibility flag enables the exact same behavior as the `nodejs_compat_v2` compatibility flag, as long as your compatibility date is set to September 23rd, 2024 or later. If you want to explicitly disable the Node.js v2 compatibility features to reduce bundle size, you can use the `no_nodejs_compat_v2` flag.
Copy link
Member

@GregBrimble GregBrimble Dec 16, 2024

Choose a reason for hiding this comment

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

Take it or leave it:

Suggested change
As of September 23rd, 2024, the `nodejs_compat` compatibility flag enables the exact same behavior as the `nodejs_compat_v2` compatibility flag, as long as your compatibility date is set to September 23rd, 2024 or later. If you want to explicitly disable the Node.js v2 compatibility features to reduce bundle size, you can use the `no_nodejs_compat_v2` flag.
As of September 23rd, 2024, the `nodejs_compat` compatibility flag enables the exact same behavior as the `nodejs_compat_v2` compatibility flag, as long as your compatibility date is set to September 23rd, 2024 or later. If you want to explicitly disable these improved Node.js compatibility features (for example, to reduce bundle size), you can use the `no_nodejs_compat_v2` flag.

@@ -50,7 +50,7 @@ You can opt into improved Node.js compatibility by using `nodejs_compat_v2` inst
additionally you can import Node.js modules without the `node:` prefix and use polyfilled Node.js modules and globals that are not available with `nodejs_compat`.

On September 23, 2024, `nodejs_compat` will use the improved Node.js compatibility currently enabled with `nodejs_compat_v2`. This will require updating your
compatibility_date to 2024-09-23 or later.
compatibility_date to 2024-09-23 or later. If you want to explicitly disable the Node.js v2 compatibility features to reduce bundle size, you can use the `no_nodejs_compat_v2` flag.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to be more explicit and say "both nodejs_compat and no_nodejs_compat_v2"

@vicb
Copy link
Contributor

vicb commented Dec 16, 2024

I'd also like to see an actual Compatibility Flag entry for this in the list below. You can create one by creating a file like ./src/content/compatibility-flags/nodejs-compat-v2.md:

---
_build:
  publishResources: false
  render: never
  list: never

name: "Node.js compatibility v2"
sort_date: "2024-09-23"
enable_date: "2024-09-23"
enable_flag: "nodejs_compat_v2"
disable_flag: "no_nodejs_compat_v2"
---

This flag, when used in conjunction with `nodejs_compat`, allows you to import Node.js modules without the `node:` prefix and will polyfill additional Node.js modules and globals that are not available with `nodejs_compat` alone.

A couple comments:

  • Sounds weird to use 2024-09-23 for nodejs_compat_v2 as it was available before?
  • "This flag, when used in conjunction with nodejs_compat" you do not need to use nodejs_compat as nodejs_compat_v2 implies nodejs_compat.

@vicb
Copy link
Contributor

vicb commented Dec 16, 2024

This code is useful to understand how this works

  nodeJsCompatV2 @50 :Bool
      $compatEnableFlag("nodejs_compat_v2")
      $compatDisableFlag("no_nodejs_compat_v2")
      $impliedByAfterDate(name = "nodeJsCompat", date = "2024-09-23");
  # Implies nodeJSCompat with the following additional modifications:
  # * Node.js Compat built-ins may be imported/required with or without the node: prefix
  # * Node.js Compat the globals Buffer and process are available everywhere

Basically using nodeJsCompat with a date >= 2024-09-23 implies nodeJsCompatV2.

Using ["nodejs_compat", "no_nodejs_compat_v2"] (after 2024-09-23 but also works before) will only enable nodeJsCompat but not nodeJsCompatV2.

(It took me some time to figure out so it might be helpful to some people reading this issue)

@GregBrimble
Copy link
Member

Sounds weird to use 2024-09-23 for nodejs_compat_v2 as it was available before?

Right, but 2024-09-23 is just the date when it is enabled by default. Would you expect something else here? Are other there any other compat flags which are more in line with whatever you're thinking?

"This flag, when used in conjunction with nodejs_compat" you do not need to use nodejs_compat as nodejs_compat_v2 implies nodejs_compat.

Ah yes, absolutely. Thanks for catching that. Lemme go edit that suggestion to be just "This flag..."

@vicb
Copy link
Contributor

vicb commented Dec 16, 2024

when it is enabled by default

It's enable by default ONLY when nodejs_compat is used. I can not really think of a good way to set the date. Buy maybe we can expand the description to be very clear about that.

@GregBrimble
Copy link
Member

Maybe a "Requires" row in that mini table? What about something like this?

image

@vicb
Copy link
Contributor

vicb commented Dec 16, 2024

Maybe a "Requires" row in that mini table? What about something like this?

image

I find it misleading as it might sound like v2 requires nodejs_compat.

IMO the only sane way to explain this would be "implied by: nodejs_compat and compatibility_date >= 2024-09-23". Not sure if we need a row, explaining this in description would work.

@vicb
Copy link
Contributor

vicb commented Dec 17, 2024

Looking at https://developers.cloudflare.com/workers/configuration/compatibility-flags/, we do not have a en entry for nodejs_compat in https://developers.cloudflare.com/workers/configuration/compatibility-flags/#flags-history.

So maybe we do not need an entry for nodejs_compat_v2 there?

There is an existing https://developers.cloudflare.com/workers/configuration/compatibility-flags/#nodejs-compatibility-flag section where we can add details.

@GregBrimble
Copy link
Member

Yeah, we really should add both here. These listings also power APIs which are used by the dashboard for selecting compat flags/dates.

@vicb
Copy link
Contributor

vicb commented Dec 17, 2024

These listings also power APIs which are used by the dashboard for selecting compat flags/dates.

I was thinking about exactly that yesterday (the dashboard list).
I don't see it as a reason to add them to the list.
What I mean is that I would prefer that:

  • those 2 flags are always added to the list (on the consumer side)
  • we keep the documentation in this existing separate section because we don't really want them to get lost in the middle of the list and they do not have a "Default as of" either.

Note that this is only thoughts no strong opinions (as long as the docs accurately describe the flags)

@GregBrimble
Copy link
Member

Yeah, whatever — I don't hugely care either way to be honest. I've said my piece :) PCX/Product can make calls about exactly how they want to describe and expose these. I'm sure it'll be fine either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product:workers Related to Workers product size/xs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add documentation for no_nodejs_compat_v2 compatibility flag
7 participants