Upgrade lockfile format, stub npm min version by cloud-walker · Pull Request #6824 · recharts/recharts

cloud-walker · 2025-12-25T17:53:17Z

Description

When a developer tries to contribute with a different npm version the generated lockfile differs, this PR aims to improve the package.json engines setting, adding a minimum npm version.

Related Issue

#6822

Motivation and Context

Avoid extra diff in new PRs just because a new contributor has different npm version installed in their machine

Summary by CodeRabbit

Documentation
- Updated development documentation with improved formatting, enhanced comments, and additional clarity around setup commands.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-25T17:53:26Z

Walkthrough

Documentation file DEVELOPING.md receives formatting improvements including additional blank lines and inline comments around existing command blocks. A node version note is added to npm install instructions. No logic or functional content changes.

Changes

Cohort / File(s)	Summary
Documentation formatting `DEVELOPING.md`	Added blank lines and inline comments around existing examples and command blocks; added node version note to npm install section

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The PR title mentions 'Upgrade lockfile format' and 'stub npm min version', but the actual changes only involve adding comments and formatting to DEVELOPING.md with no functional changes to lockfile format or npm version settings in package.json.	Update the title to accurately reflect the actual changes, such as 'Add npm minimum version requirement documentation' or 'Document npm version requirements in DEVELOPING.md'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description addresses the motivation and related issue well, but lacks detail on 'How Has This Been Tested' and 'Types of changes' sections required by the template.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d05157a and 6252197.

⛔ Files ignored due to path filters (1)

package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (1)

DEVELOPING.md

🧰 Additional context used

🧠 Learnings (20)

📓 Common learnings

Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6824
File: package.json:0-0
Timestamp: 2025-12-25T23:55:31.361Z
Learning: In the recharts repository, the `engines` field in package.json specifies requirements for library consumers and should only be changed with major version releases to avoid breaking changes. For development environment Node version requirements, use the `.nvmrc` file instead, which standardizes the development environment without impacting library consumers.

📚 Learning: 2025-12-25T23:55:31.361Z

Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6824
File: package.json:0-0
Timestamp: 2025-12-25T23:55:31.361Z
Learning: In the recharts repository, the `engines` field in package.json specifies requirements for library consumers and should only be changed with major version releases to avoid breaking changes. For development environment Node version requirements, use the `.nvmrc` file instead, which standardizes the development environment without impacting library consumers.

Applied to files:

DEVELOPING.md

📚 Learning: 2025-12-06T03:36:59.377Z

Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-12-06T03:36:59.377Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : All imports from `recharts` must use the public API entry point; imports from internal paths like `recharts/types/*` or `recharts/src/*` are not allowed

Applied to files:

DEVELOPING.md

📚 Learning: 2025-12-16T08:12:13.355Z

Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6783
File: test/util/ChartUtils.spec.tsx:15-16
Timestamp: 2025-12-16T08:12:13.355Z
Learning: In the recharts codebase, files in the `test` folder are allowed to import from internal paths (e.g., `../../src/state/cartesianAxisSlice`) and do not need to use the public API entry point (`src/index.ts`). The public API import restriction applies only to non-test code.

Applied to files:

DEVELOPING.md

📚 Learning: 2025-12-06T03:36:59.377Z

Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-12-06T03:36:59.377Z
Learning: Run type checking with `npm run check-types` to verify TypeScript types

Applied to files:

DEVELOPING.md

📚 Learning: 2025-11-23T13:30:10.395Z

Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6669
File: www/src/docs/exampleComponents/ScatterChart/ScatterChartWithLabels.tsx:2-2
Timestamp: 2025-11-23T13:30:10.395Z
Learning: The `TooltipIndex` type from recharts is defined in `src/state/tooltipSlice.ts` but is not currently exported from the public API entry points. It should not be imported from `recharts/types/state/tooltipSlice` as this is an internal implementation path. An ESLint rule is needed to prevent regressions.

Applied to files:

DEVELOPING.md

📚 Learning: 2025-12-14T13:58:58.361Z

Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6771
File: src/shape/Curve.tsx:39-40
Timestamp: 2025-12-14T13:58:58.361Z
Learning: In the recharts codebase, `useAppSelector` and hooks like `useChartLayout()` are designed to return `undefined` when used outside Redux Provider context, rather than throwing errors. This allows components like `Curve` that call these hooks to work standalone by falling back to prop values.

Applied to files:

DEVELOPING.md

📚 Learning: 2025-12-08T08:23:26.261Z

Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6750
File: src/state/selectors/axisSelectors.ts:593-602
Timestamp: 2025-12-08T08:23:26.261Z
Learning: In the recharts codebase, `DataKey<any>` is an intentional exception to the "no any" rule while proper typing is being developed. This should not be flagged in reviews.

Applied to files:

DEVELOPING.md

📚 Learning: 2025-11-25T01:22:59.729Z

Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.729Z
Learning: Applies to **/*.{ts,tsx} : Type function parameters and return values explicitly in TypeScript, do not rely on implicit any or inference; exceptions are React components and trivial functions

Applied to files:

DEVELOPING.md

📚 Learning: 2025-11-25T01:22:59.729Z

Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.729Z
Learning: Applies to **/*.{ts,tsx} : Do not use `as` type assertions in TypeScript; the only exception is `as const`

Applied to files:

DEVELOPING.md

📚 Learning: 2025-11-19T14:08:01.728Z

Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6659
File: www/src/components/GuideView/Performance/index.tsx:232-250
Timestamp: 2025-11-19T14:08:01.728Z
Learning: In Recharts version 3.4.2, object-as-prop optimizations were introduced to reduce unnecessary re-renders when new object references are passed as props. This affects the recommendation for the `react-perf/jsx-no-new-object-as-prop` ESLint rule.

Applied to files:

DEVELOPING.md

📚 Learning: 2025-11-25T01:22:59.729Z

Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.729Z
Learning: Applies to **/*.{ts,tsx} : Never use `any` type (implicit or explicit) in TypeScript code

Applied to files:

DEVELOPING.md

📚 Learning: 2025-11-25T01:23:14.977Z

Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:23:14.977Z
Learning: Applies to **/*.spec.{ts,tsx} : When running unit tests, prefer to run a single test file using `npm run test -- path/to/TestFile.spec.tsx` rather than running all tests with `npm test`

Applied to files:

DEVELOPING.md

📚 Learning: 2025-11-25T01:23:08.250Z

Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Aim for 100% unit test code coverage when writing new code

Applied to files:

DEVELOPING.md

📚 Learning: 2025-11-25T01:22:59.729Z

Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.729Z
Learning: Applies to test/**/*.spec.{ts,tsx} : Aim for 100% unit test code coverage when writing new code

Applied to files:

DEVELOPING.md

📚 Learning: 2025-11-25T01:22:59.729Z

Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.729Z
Learning: Ensure the test suite passes by running `npm run test` before submitting a pull request

Applied to files:

DEVELOPING.md

📚 Learning: 2025-11-25T01:22:59.729Z

Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.729Z
Learning: Applies to test/component/**/*.spec.tsx : Use React Testing Library for testing component interactions and behavior upon rendering

Applied to files:

DEVELOPING.md

📚 Learning: 2025-11-25T01:22:59.729Z

Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.729Z
Learning: Applies to storybook/stories/**/*.stories.tsx : Use Storybook for smoke tests and add play functions with assertions for actual tests

Applied to files:

DEVELOPING.md

📚 Learning: 2025-11-25T01:23:08.250Z

Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Prefer to use the `createSelectorTestCase` helper function when writing or modifying tests

Applied to files:

DEVELOPING.md

📚 Learning: 2025-11-25T01:23:08.250Z

Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Use `vi.useFakeTimers()` in all tests due to Redux autoBatchEnhancer dependency on timers and `requestAnimationFrame`

Applied to files:

DEVELOPING.md

🪛 markdownlint-cli2 (0.18.1)

DEVELOPING.md

49-49: Dollar signs used before commands without showing output

(MD014, commands-show-output)

🔇 Additional comments (2)

DEVELOPING.md (2)

9-9: Good alignment with development environment practices.

The inline comment pointing to .nvmrc for the correct Node version is helpful and aligns with best practices—the .nvmrc file is the proper place for development environment Node version requirements, separate from the package.json engines field which targets library consumers. This will reduce confusion and ensure developers use the intended development version.

26-26: Formatting improvements enhance readability.

The added blank lines before code blocks and bullet points improve the visual structure and readability of the documentation without changing any content.

Also applies to: 31-31, 47-47, 53-53, 232-232, 235-235

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d52345f and f32417f.

⛔ Files ignored due to path filters (1)

package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (1)

package.json

package.json

…and npm 10

cloud-walker · 2025-12-26T10:42:34Z

totally missed the .nvmrc file! I'll transform this PR pointing from DEVELOPING.md that the node dev version can be found in the .nvmrc file, thanks!

codecov · 2025-12-26T11:54:44Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.56%. Comparing base (d52345f) to head (6252197).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6824   +/-   ##
=======================================
  Coverage   93.56%   93.56%           
=======================================
  Files         523      523           
  Lines       46462    46462           
  Branches     5055     5055           
=======================================
  Hits        43472    43472           
  Misses       2983     2983           
  Partials        7        7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ckifer

Technically we should be able to run nvm use and it will use the version in the nvmrc file

Following best practice demonstrated in Recharts PR #6824: - Add engines.node >= 20 to document minimum Node version - Add engines.npm >= 10 to prevent lockfile churn This ensures contributors use consistent npm versions, avoiding unnecessary diffs from lockfile format mismatches (v2 vs v3). See: recharts/recharts#6824

cloud-walker marked this pull request as ready for review December 25, 2025 17:53

cloud-walker added 2 commits December 25, 2025 18:54

Update lockfile

a293bc6

Upgrade node to at least lts

d05157a

cloud-walker force-pushed the fix-lock branch from f32417f to d05157a Compare December 25, 2025 17:55

coderabbitai bot reviewed Dec 25, 2025

View reviewed changes

package.json Outdated Show resolved Hide resolved

cloud-walker marked this pull request as draft December 25, 2025 19:41

cloud-walker changed the title ~~Upgrade lockfile format, stub npm min version and upgrade node min version~~ Upgrade lockfile format, stub npm min version Dec 25, 2025

chore: update engine requirements in package.json to support Node 18 …

7bbfdde

…and npm 10

cloud-walker added 2 commits December 26, 2025 11:48

Add comment on DEVELOPING to notify of the correct node version

c0b9fff

revert npm version for package users

6252197

cloud-walker requested a review from PavelVanecek December 26, 2025 10:54

cloud-walker marked this pull request as ready for review December 26, 2025 10:55

PavelVanecek approved these changes Dec 26, 2025

View reviewed changes

ckifer approved these changes Dec 26, 2025

View reviewed changes

ckifer merged commit 75af482 into recharts:main Dec 26, 2025
67 of 70 checks passed

cloud-walker deleted the fix-lock branch December 26, 2025 16:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Upgrade lockfile format, stub npm min version#6824

Upgrade lockfile format, stub npm min version#6824
ckifer merged 5 commits intorecharts:mainfrom
cloud-walker:fix-lock

cloud-walker commented Dec 25, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 25, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

cloud-walker commented Dec 26, 2025

Uh oh!

codecov bot commented Dec 26, 2025 •

edited

Loading

Uh oh!

ckifer left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

cloud-walker commented Dec 25, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Motivation and Context

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cloud-walker commented Dec 26, 2025

Uh oh!

codecov bot commented Dec 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ckifer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cloud-walker commented Dec 25, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 25, 2025 •

edited

Loading

codecov bot commented Dec 26, 2025 •

edited

Loading