chore: add Node Releases JSON to bundle

[HinataKah0]

Result: #5443 (comment)

This PR will include Node releases data JSON in the bundle. Based on our experiment, we found that fetching the JSON file in Client Side harms UX badly in slow 3G network. Not to mention that the bundle size increment is tiny and it takes a very long time for it to grow and become harmful.

We added node-releases-data.json containing an empty array [] as placeholder. This file will be overwritten during Build time and we don't want to commit unnecessary changes. However, once a file exist in the repository, .gitignore won't work anymore. As a workaround, we will have a pre-commit hook to discard the changes.

We use husky for pre-commit hooks (we followed these steps).

Related to: #5438

[vercel]

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

Name	Status	Preview	Comments	Updated (UTC)
nodejs-org	❌ Failed (Inspect)			Mar 19, 2024 3:05pm
nodejs-org-stories	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 19, 2024 3:05pm

[vercel]

@HinataKah0 is attempting to deploy a commit to the OpenJS Foundation Team on Vercel.

A member of the Team first needs to authorize it.

[HinataKah0]

Can I get someone to approve the deployment (Not PR)?
So I want to test on the actual environment since I am not sure if the compression is done different locally, etc...

[ovflowd]

@HinataKah0 I just approved your deployment. Sorry for the delay!

[mikeesto]

And can someone help me to try pulling my branch and check if the node-releases-data.json is actually ignored or not. Git is quite picky about it. 👀

It is being tracked atm. You should be able to do git rm --cached public/node-releases-data.json to remove the file from Git's index, and then commit the change

[ovflowd]

FYI IMHO you should actually do the following:

1. Remove the file public/node-release-data.json from .gitignore
2. Delete the file and recreate it only with this values [] (empty array)
3. Add the file and commit
4. Then git update-index --skip-worktree public/node-release-data.json

This will tell git to ignore future changes to this file.

[HinataKah0]

Then git update-index --skip-worktree public/node-release-data.json

If I understand correctly, this is only available in local machine.
Then, for others the file is still tracked.

Edit:
I am trying to google how others are usually doing this...

[HinataKah0]

Included in the bundle:

Client Side:

Bundle impact is very minimal (as expected). It doesn't show up in bundle analyzer as well. 🤣

I tried simulating slow 3G network. And Client Side rendering performs really bad.

[ovflowd]

If I understand correctly, this is only available in local machine.
Then, for others the file is still tracked.

Incorrect. The command above applies to the repository. (Doesn't matter if local, others or upstream) as soon as they pull this commit it will affect them.

[HinataKah0]

Client Side:

Screen.Recording.2023-06-20.at.8.43.20.PM.mov

126kB (compressed) -> 441kB (uncompressed)

Included in the bundle:

Screen.Recording.2023-06-20.at.8.45.30.PM.mov

127kB (compressed) -> 435kB (uncompressed) (I can't explain why this is lower uncompressed)

I feel that it's far better to include the Node Releases data in the bundle for now...
Since there isn't any real issue with the bundle size I believe (I googled it and it seems that the rule of thumb is to have it less than 300kB compressed).

[HinataKah0]

After fighting with Git for some time...

I tried both approaches described above and some other combinations but nothing work 😅
I tried searching online as well, it seems that git update-index --skip-worktree works locally but not remote (this)

I think we just don't want to upset TS, right?
This works, TS is not upset with it. And not to mention that the file is going to be generated during build time so, I believe this will be okay.
Caveat: just handle the case if it's undefined/null.

Edit:
We can also declare the NodeReleaseJSON type inside the .d.ts file, it looks neat.

What do you all think?

[ovflowd]

Edit:
We can also declare the NodeReleaseJSON type inside the .d.ts file, it looks neat.

Not really needed.

I tried both approaches described above and some other combinations but nothing work 😅
I tried searching online as well, it seems that git update-index --skip-worktree works locally but not remote (this)

Not sure what you're doing wrong. Also not sure what behaviour is happening for you. I also have no idea what exactly you're doing. It would be interesting if you could write the steps you're doing.

[ovflowd]

I think we just don't want to upset TS, right?
This works, TS is not upset with it. And not to mention that the file is going to be generated during build time so, I believe this will be okay.
Caveat: just handle the case if it's undefined/null.

Not sure what you're saying. The JSON file type is resolved implicitly by TypeScript itself, so no need to define types for the JSON. The Provider itself has a type, and if you pass the mapped data it should also be all good.

If that is still not enough you can create a type for an individual entry of the array and pass it within the callback nodeReleaseData.map((release: NodeReleaseSource) => ...)

[HinataKah0]

Oops, I just typed it out without re-reading again. 😅
Let me clarify things.

So, I tried exactly these steps:
#5443 (comment)

1. Deleted public/node-releases-data.json from .gitignore
2. Deleted the local JSON file & recreated the JSON file again with empty array []
3. Committed it fda3146
4. Ran git update-index --skip-worktree public/node-releases-data.json

It works in my local (the JSON file is not tracked).
I tried cloning my fork again in another directory, but the JSON file is still tracked.

I tried the step suggested by Mike as well but it also doesn't work.
Did step 1-3 again + Added back the JSON file to .gitignore + Ran git rm --cached public/node-releases-data.json
The file is still tracked.

I've tried other combinations of steps as well but none is working. Not sure if it just happened to me or not. 😅

Then, I checked online, it seems that git update-index --skip-worktree only works locally.

I believe our concern of having the JSON file with empty array [] is because we don't want to upset TypeScript. I checked for another alternative: f9db6e7

[ovflowd]

I believe our concern of having the JSON file with empty array [] is because we don't want to upset TypeScript. I checked for another alternative: f9db6e7

What if after committing the file with the empty array you re-add the file to the gitignore?

[HinataKah0]

The step suggested by Mike will delete the actual file in remote, see here (talking about this outdated comment above)

What if after committing the file with the empty array you re-add the file to the gitignore?

I tried it as well...
It is tracked if the file has been committed (although it is inside gitignore)

[ovflowd]

I tried it as well...
It is tracked if the file has been committed (although it is inside gitignore)

What about this: https://stackoverflow.com/a/26245961

[HinataKah0]

What about this: https://stackoverflow.com/a/26245961

Tried it, also doesn't work...
https://stackoverflow.com/questions/18570926/do-git-update-index-assume-unchanged-rules-propagate-to-clients-on-pull#:~:text=The%20index%20is%20local%20to,clones%20of%20the%20same%20remote.

[ovflowd]

@HinataKah0 did you follow Husky getting started steps correctly? The package.json should have been updated with a prepare step

[ovflowd]

NVM, just saw it.

[HinataKah0]

I followed these steps: https://typicode.github.io/husky/getting-started.html#automatic-recommended

[ovflowd]

FYI I've pushed some little changes to your branch to fix some things. (Like Husky)

[ovflowd]

LGTM! Good stuff, @HinataKah0!

[ovflowd]

BTW nvm the PR failures, that's something else that I'm patching on main. (And then undo the nextFonts change on this PR)

[HinataKah0]

Thanks!
I saw the changes about the types. I'll verify them tmr.
Edited (because this PR has been merged, I don't want to make noise):
I've verified the changes, it looks good! I just knew about git sparse as well, thanks! 👀

FYI, my original intention of keeping different types (NodeReleaseJSON and NodeRelease) was because:
In useFetchNodeReleases (now it is moved to be inside the Provider itself), we are populating the empty fields with empty string ''. So in NodeReleaseJSON type, some fields are not guaranteed to exist but in NodeRelease they are.

I am okay with the new abstractions (NodeRelease extends NodeReleaseSource). I think it's neater.