chore: consolidate Node releases data

[HinataKah0]

Design

1. In Build time, generate a static JSON file containing all Node releases (filtered & destructured nodevu's outputs which will greatly reduced the generated JSON file size).
2. In Client side, fetch the generated static JSON file using useSWR hook, do formatting & time sensitive logic (e.g. computing the release status).
3. Then, the ready-to-consume data is passed through the component tree using Context & Provider.
4. Implement WithNodeRelease component which acts as a guard (not to render a component if can't find a specific Node release, e.g. LTS or Current).

Legacy useNodeData hook will be removed and legacy usages in nodejs.org will be updated.

Update (27/06/2023):
#5443
This PR includes Node releases data in the bundle. Fetching the data in Client side harms the UX badly in slow 3G network.

TODO

• Once this PR is merged, write a guide on how to leverage WithNodeRelease in the Download Components Migration issue.

Related to: #5350

(WIP) Guide on Node Releases Data

Provider

useNodeReleases

If you need all the past Node Releases, here's how to use this hook:

const { releases } = useNodeReleases();
{releases.map(release => (
  ...

Example: DownloadReleasesTable.tsx

If you need only a single Node Release (usually the latest LTS/Current), you can use WithNodeRelease to wrap your component (see below for details).

WithNodeRelease

There are many cases requiring us to provide a Component with a single NodeRelease object. Usually it is the latest Node release with either LTS or Current status (e.g. this, and this). You can get the desired NodeRelease object using WithNodeRelease "Provider", see below for how to use it.

It is not encouraged to create a new Props type if it contains only a single NodeRelease object, instead you can use NodeRelease as the Props type itself.

const MyComponent: FC<NodeRelease> = ({
  version,
  versionWithPrefix,
  isLts,
  npm,
}) => {

...

<WithNodeRelease status="Active LTS">
  {({ release }) => <MyComponent {...release} />}
</WithNodeRelease>

Example:

• Parent Child (MyComponent)
• Parent Child (MyComponent) -> more flexible variation

In case you need other Props, here's how you can pass additional Props.

type MyComponentProps = {
  release: NodeRelease;
  anotherProp: string;
};

const MyComponent: FC<MyComponentProps> = ({
  release: {version, versionWithPrefix, isLts, npm}, // You can destructure if it is appropriate
  anotherProp,
}) => {

...

<WithNodeRelease status="Current">
  {({ release }) => <MyComponent release={release} anotherProp="Another Prop" />}
</WithNodeRelease>

How to Mock (for Tests)?

WIP
Refer to this comment for the time being

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
nodejs-org	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 17, 2023 8:20am
nodejs-org-stories	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 17, 2023 8:20am

[ovflowd]

I see that you're recreating a few things here.

Let's please simply update the existing file https://github.com/nodejs/nodejs.org/blob/main/scripts/next-data/getNodeVersionData.mjs to do this logic.

You can also remove all the route logic from there.

This also means you can remove this https://github.com/nodejs/nodejs.org/blob/main/next.data.mjs#L18 and https://github.com/nodejs/nodejs.org/blob/main/next.data.mjs#L21, as the nodeReleaseData will not be part of each page context. And call the function...

And call it around here https://github.com/nodejs/nodejs.org/blob/main/next.data.mjs#L7

You also need not update package.json with an extra script/step.

This also means you should probably remove the usages of https://github.com/search?q=repo%3Anodejs%2Fnodejs.org%20nodeVersionData&type=code that comes from the App context.

Yes, this change would effectively also change/affect the current website (not just website-redesign).

So honestly speaking, all these changes should be done directly targeting the main branch.

You can

[HinataKah0]

Thank you for the inputs!

Let me summarise to verify that I understood correctly.

1. Basically we are replacing this file to generate the JSON file. Replacing because I want to rename the file.
2. The generated JSON file will fulfil both nodejs.org and nodejs.dev requirements.
3. Then, we will remove NodeVersionData, so existing nodejs.org pages will fetch data from this generated JSON file using NodeReleaseData structure (we need to add some new fields -> dependencies version).

Initially I was hesitant because I wanted to maintain clear separation (not touching existing things).
+1 I think it should be okay to update existing pages as well, so we can consolidate everything thus it's less confusing.

[HinataKah0]

@ovflowd I've updated the branch with latest main as well...

Can we let it bake for a while?
Let's deploy in Vercel & I'll play around with it...

[ovflowd]

Sure, @HinataKah0 I'm going to run the deployment and then you can start testing it :)

[ovflowd]

LGTM (just another approval) but just wanted to reiterate the stellar work you've done here!!

[HinataKah0]

I've done some sanity checks, looks good so far :)

[ovflowd]

@HinataKah0 do you know if the legacyMain.js is still responsible for generating this header? Because the substitution of "Download for" to "Download for macOS" happens before the actual WithNodeRelease happens. (Screenshot of the Heading)

And a recording of the issue (with a throttled network/simulated network condition just to show the racing-condition)

Screen.Recording.2023-06-15.at.00.17.37.mov

[HinataKah0]

I think it's expected because:

"Download" -> because the default OS is OTHER
"Download for macOS" -> useDetectOS hook finishes execution
And after Node releases data is fetched, both buttons are rendered...

with a throttled network/simulated network condition just to show the racing-condition

Just curious, are you putting timeout or there's convenient way?

[ovflowd]

Just curious, are you putting timeout or there's convenient way?

I've used the DevTools to create a "metered network condition" of "slow 3g"

"Download" -> because the default OS is OTHER
"Download for macOS" -> useDetectOS hook finishes execution
And after Node releases data is fetched, both buttons are rendered...

That honestly makes sense. Since useDetectOS is independent of the NodeReleaseProvider.

I think that even with the NodeReleaseData being pre-renderer we would still have this "issue" on the current layout.

Luckily the new design (https://nodejs.dev/en/) doesn't has such pitfall, since the Download Section (OS selection) is aggregated together with the Download buttons, making the render already be conditional once both Release Data + UserOS got defined. So not something we need to worry about as it is more an UX issue that is already going to be solved by the new design.

[HinataKah0]

@nodejs/website I'll let this bake for some time and please help for another approval... Thanks!

[ovflowd]

@Harkunwar if you could give a review here that'd be awesome!

[ovflowd]

@HinataKah0 the new rules got accepted so if you want you can merge this PR :) But if you want to wait for another review, feel free.

[mikeesto]

LGTM - nice work

[ovflowd]

Going to proceed with merging this for now :) if we need to make amends, let me know.

[HinataKah0]

🚀

It has been merged, and it has been a long ride!
Thanks for all the awesome inputs Claudio, I have learnt many new things along the process.
Thanks Augustin for the input on fixtures, I really appreciate that.
Thanks Michael for the final review as well!

I really appreciate all the helps in making this done!
I'll update the PR description gradually (since writing has never been my forte 🙈 ).
But I think people can start referring to it now for working on the Download Components.