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

chore: update package.json files with repository fields #29582

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

Conversation

skratchdot
Copy link
Contributor

@skratchdot skratchdot commented May 24, 2024

Summary

I recently learned about eslint-plugin-react-compiler so wanted to try it in one of my projects. I ran npm repo eslint-plugin-react-compiler to find out more (and see where I could submit a PR for a small issue I found), but that did not work. It took me a while to find the source code.

I decided to make a small script to add a "repository" field to all the package.json in this repo. After we merge this PR, and publish new packages, then npm repo will work for the newly published packages.

I spent a little time to ignore certain private package.json files (that didn't need the "repository" field):
https://github.com/facebook/react/pull/29582/files#diff-c38afa3bfcedbfb5d75f077f1449459fcc0044ed09a12f75f6b93d0dd16f4f4fR30-R34

I also added some extra logic to include the new "repository" field before "scripts" (if it exists) or else "dependencies" (if it exists) or else at the bottom of the file.

Hopefully this PR is minimal enough to be accepted. Let me know if you have any questions/concerns!

All the package.json changes were made by running node scripts/tasks/update-package-files.js in the root folder.

If there is no need to ever use this script again, I can remove it from the PR, and only submit the package.json changes.

How did you test this change?

I ran the script a few times while working on it (using git diff and git checkout to confirm my changes and re-run from a fresh state, etc).

Running the script should be "run twice safe" (idempotent) and the output looks like:

➜ node scripts/tasks/update-package-files.js
Updating 1 of 45: compiler/apps/playground/package.json
Updating 2 of 45: compiler/crates/react_napi/package.json
Updating 3 of 45: compiler/package.json
Updating 4 of 45: compiler/packages/babel-plugin-react-compiler/package.json
Updating 5 of 45: compiler/packages/eslint-plugin-react-compiler/package.json
Updating 6 of 45: compiler/packages/make-read-only-util/package.json
Updating 7 of 45: compiler/packages/react-compiler-healthcheck/package.json
Updating 8 of 45: compiler/packages/react-compiler-runtime/package.json
Updating 9 of 45: compiler/packages/snap/package.json
Updating 10 of 45: packages/eslint-plugin-react-hooks/package.json
Updating 11 of 45: packages/jest-react/package.json
Updating 12 of 45: packages/react-art/package.json
Updating 13 of 45: packages/react-cache/package.json
Updating 14 of 45: packages/react-client/package.json
Updating 15 of 45: packages/react-debug-tools/package.json
Updating 16 of 45: packages/react-devtools-core/package.json
Updating 17 of 45: packages/react-devtools-extensions/package.json
Updating 18 of 45: packages/react-devtools-fusebox/package.json
Updating 19 of 45: packages/react-devtools-inline/package.json
Updating 20 of 45: packages/react-devtools-shared/package.json
Updating 21 of 45: packages/react-devtools-shell/package.json
Updating 22 of 45: packages/react-devtools-timeline/package.json
Updating 23 of 45: packages/react-devtools/package.json
Updating 24 of 45: packages/react-dom-bindings/package.json
Updating 25 of 45: packages/react-dom/package.json
Updating 26 of 45: packages/react-is/package.json
Updating 27 of 45: packages/react-native-renderer/package.json
Updating 28 of 45: packages/react-noop-renderer/package.json
Updating 29 of 45: packages/react-reconciler/package.json
Updating 30 of 45: packages/react-refresh/package.json
Updating 31 of 45: packages/react-server-dom-esm/package.json
Updating 32 of 45: packages/react-server-dom-fb/package.json
Updating 33 of 45: packages/react-server-dom-turbopack/package.json
Updating 34 of 45: packages/react-server-dom-webpack/package.json
Updating 35 of 45: packages/react-server/package.json
Updating 36 of 45: packages/react-suspense-test-utils/package.json
Updating 37 of 45: packages/react-test-renderer/package.json
Updating 38 of 45: packages/react/package.json
Updating 39 of 45: packages/scheduler/package.json
Updating 40 of 45: packages/use-subscription/package.json
Updating 41 of 45: packages/use-sync-external-store/package.json
Updating 42 of 45: scripts/bench/package.json
Updating 43 of 45: scripts/devtools/package.json
Updating 44 of 45: scripts/perf-counters/package.json
Updating 45 of 45: scripts/release/package.json

Copy link

vercel bot commented May 24, 2024

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

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 17, 2024 0:11am

@@ -1,20 +1,23 @@
{
"name": "@react/forget-napi",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this seems to be the only package.json in the repo using "4 spaces", so it's the biggest diff. the only change should be the new repository field.

viewing the diff with "hide whitespace" makes this more clear :)

@react-sizebot
Copy link

react-sizebot commented May 24, 2024

Comparing: f14d7f0...309d3cf

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.66 kB 6.66 kB = 1.82 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 497.93 kB 497.93 kB = 89.26 kB 89.26 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 502.75 kB 502.75 kB = 89.96 kB 89.96 kB
facebook-www/ReactDOM-prod.classic.js = 597.17 kB 597.17 kB = 105.33 kB 105.33 kB
facebook-www/ReactDOM-prod.modern.js = 571.52 kB 571.52 kB = 101.27 kB 101.27 kB
test_utils/ReactAllWarnings.js Deleted 62.88 kB 0.00 kB Deleted 15.69 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
test_utils/ReactAllWarnings.js Deleted 62.88 kB 0.00 kB Deleted 15.69 kB 0.00 kB

Generated by 🚫 dangerJS against 309d3cf

@kachkaev
Copy link

kachkaev commented Jun 9, 2024

Thanks for your contribution @skratchdot! I was about to add repository etc. to package.json files too, and I’m glad that I’ve discovered your PR first! This metadata is quite important: it lets us navigate from npmjs.com to GitHub etc. It’s currently impossible for, say, eslint-plugin-react-compiler:

With your PR merged, we’ll see new links like these:

Underlying metadata:

"homepage": "https://react.dev/",
"bugs": "https://github.com/facebook/react/issues",

"repository": {
"type": "git",
"url": "https://github.com/facebook/react.git",
"directory": "packages/react"
},

A diff with a script may take a bit longer to review, so if splitting this PR into two helps, it’d be great if maintainers could share their opinion. In theory, we can merge package.json changes first and then chuck in a script that optimises all future changes.

@skratchdot
Copy link
Contributor Author

A diff with a script may take a bit longer to review, so if splitting this PR into two helps, it’d be great if maintainers could share their opinion. In theory, we can merge package.json changes first and then chuck in a script that optimises all future changes.

Thanks for the comments and for taking a look! I was hesitant to add the script to the PR, but ended up doing it anyways. It might make more sense to make it a gist and reference it in the PR 🤷. I don't like the idea of adding unnecessary bloat to the repo (especially if the script has to be manually ran- and is only useful if new packages are added).

I think adding homepage and bugs fields would be great/easy follow-up PRs (or I can even add them to this one). It should be easy to modify the script to add those.

I'll wait for comments from the maintainers before doing anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants