Skip to content

Fix resolve install in old npm version on CI.#3237

Open
rasmi wants to merge 1 commit intoimport-js:mainfrom
rasmi:resolver-install-nested-deps
Open

Fix resolve install in old npm version on CI.#3237
rasmi wants to merge 1 commit intoimport-js:mainfrom
rasmi:resolver-install-nested-deps

Conversation

@rasmi
Copy link
Copy Markdown

@rasmi rasmi commented Mar 12, 2026

On Node < 10 in CI, linklocal replaces the published resolver with a symlink to the local source, but old npm doesn't install the local source's nested dependencies, so resolve@2 (added in 9222532) is missing and the resolver crashes. The fix adds (cd resolvers/node && npm install) to dep-time-travel.sh to install the missing nested deps before tests run.

Also, extends the timeout for the first webpack resolver test on old node + macos because it's a bit over the current 5s timeout. The initial load here takes longer.

@rasmi rasmi mentioned this pull request Mar 12, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.50%. Comparing base (9222532) to head (2bd1e28).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3237      +/-   ##
==========================================
+ Coverage   95.39%   95.50%   +0.10%     
==========================================
  Files          83       83              
  Lines        3689     3689              
  Branches     1332     1332              
==========================================
+ Hits         3519     3523       +4     
+ Misses        170      166       -4     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment on lines +48 to +49
echo "Installing resolver dependencies..."
(cd resolvers/node && npm install)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this should probably run for every subdir inside resolvers, so it covers the webpack one too

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done.

@ljharb ljharb marked this pull request as draft March 16, 2026 17:24
@rasmi
Copy link
Copy Markdown
Author

rasmi commented Mar 16, 2026

Extended timeout on the one failing test for old macos as it just takes a little longer for the initial webpack resolver load.

@rasmi rasmi marked this pull request as ready for review March 16, 2026 22:15
@danielhjacobs
Copy link
Copy Markdown

Since it was already approved, what is stopping this from being merged to unblock #3230 to unblock jsx-eslint/eslint-plugin-react#3979?

@ljharb ljharb force-pushed the resolver-install-nested-deps branch from 2bd1e28 to 154a5e0 Compare April 2, 2026 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants