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

nodejs_compat(_v2), clarification + factor docs #18773

Closed
vicb opened this issue Dec 16, 2024 · 4 comments · Fixed by #19079
Closed

nodejs_compat(_v2), clarification + factor docs #18773

vicb opened this issue Dec 16, 2024 · 4 comments · Fixed by #19079
Assignees
Labels
content:edit Request for content edits documentation Documentation edits

Comments

@vicb
Copy link
Contributor

vicb commented Dec 16, 2024

Related to #18765

The docs refers to nodejs_compat(_v2) quite a lot - it seems like the content is repeated each time. So some of the docs are not consistent / still refer to nodejs_compat_v2

This page has been updated but all the other pages in the same subtree have not, i.e. this page

It would be nice to have a centralized snippter that we could <Render file="path/to/nodejs_compat" product="workers" />

/cc @ToriLindsay

@ToriLindsay
Copy link
Contributor

Hi @vicb - Just have a few follow-up questions here:

  1. Is that first link in your description meant to go here? It's just leading me to a GH page.
  2. Just to confirm - Are you suggesting that we change all places where it says nodejs_compat_v2 to simply nodejs_compat (since it enables v2 by default) ? (Excluding situations where we need to call out v2 specifically, as in Added no_nodejs_compat_v2 to notes #18765 ) ? Just making sure I understand.
  3. And then what would the partial (the snippet) be of exactly? A warning/note as it is here ?

@vicb
Copy link
Contributor Author

vicb commented Dec 17, 2024

  1. you are right, sorry for the broken link - I updated the original description
  2. There should be no reason to use nodejs_compat_v2 any more (unless the project is stuck to a compat date before 2024-09-23) - Using no_nodejs_compat_v2 might make sense though i.e. to disable bundling.
  3. I think the partial should be a brief note describing how to enable the compatibility mode (either nodejs_compat after 2024-09-23 or nodejs_compat_v2) and a link to a page with all the details - probably here

Happy to chat about this later this week if needed.

@ToriLindsay
Copy link
Contributor

@vicb
FYI - I created #19079 for this. Sam already approved it so I will merge soon, but feel free to review anyway. Those docs you linked that mentioned v2 were already referencing a partial so I just updated that partial. But I also found a couple more instances of docs mentioning v2 which I updated as well. I'm also planning to follow this up with a rewrite of this section which will explain the difference between v1 and v2 more clearly. I will post that in #18765
cc: @mikenomitch

@vicb
Copy link
Contributor Author

vicb commented Jan 10, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content:edit Request for content edits documentation Documentation edits
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants
@vicb @pedrosousa @dcpena @haleycode @patriciasantaana @ToriLindsay and others