Opened 6 months ago
Last modified 5 days ago
#62839 reopened defect (bug)
sync-gutenberg-packages script is not respecting .npmrc file
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | 6.9 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Build/Test Tools | Keywords: | has-patch has-test-info changes-requested |
Focuses: | Cc: |
Description
It seems that running npm sync-gutenberg-packages
results in some packages being duplicated within the package-lock.json
file despite prefer-dedupe
being set to true
in .npmrc
.
To test, run the following steps and committing after each one to a test branch, or staging changes so further changes are trackable:
- run
npm dedupe
ontrunk
to properly dedupe. - run
npm sync-gutenberg-packages
. - run
npm dedupe
.
There is also the following error when running sync-gutenberg-packages
:
wordpress-develop/tools/release/sync-gutenberg-packages.js:110
const versionConflicts = Object.entries( packageLock.dependencies )
TypeError: Cannot convert undefined or null to object
This may be related to ticket:62190#comment:17 from @kevin940726.
Change History (12)
This ticket was mentioned in Slack in #core by johnbillion. View the logs.
6 months ago
This ticket was mentioned in Slack in #core by johnbillion. View the logs.
5 months ago
This ticket was mentioned in PR #8431 on WordPress/wordpress-develop by jtquip88.
4 months ago
#3
- Keywords has-patch added; needs-patch removed
#4
@
3 months ago
- Keywords needs-testing added
- Milestone changed from 6.8 to 6.9
Unfortunately someone did not get the time to review this leading up to 6.8.
#5
@
3 months ago
- Keywords needs-testing-info reporter-feedback added; needs-testing removed
Combined Issue Reproduction and Test Report
Description
🟠 This report can't validate much about this whole issue
Patch tested: https://github.com/WordPress/wordpress-develop/pull/8431.diff
Environment
- WordPress: 6.9-alpha-60093-src
- PHP: 8.2.28
- Server: nginx/1.27.4
- Database: mysqli (Server: 8.0.41 / Client: mysqlnd 8.2.28)
- Browser: Chrome 134.0.0.0
- OS: Windows 10/11
- Theme: Twenty Twenty-Five 1.1
- Plugins:
- Test Reports 1.2.0
Reproduction Steps
- Run
npm dedupe --verbose
(without verbosity, you cannot see if it gets stuck at some point), I can't see any changes after this. - ❌ Run
npm run sync-gutenberg-packages
. File changes, but I can't confirm any duplicates. - 🐞 Can confirm the TypeError during
sync-gutenberg-packages
- ❌ After each
dedupe
I can't confirm any new changes inpackage-lock.json
. I can't confirm any duplicates either.
Expected
- Given the absence of duplicates, I'm not 100% sure what to expect
- The error is gone
Actual Results with the Patch
- ✅ The error is gone with the patch.
- 🟠 No real impact done by this patch, I can't see a single difference between
package-lock.json
before and after the patch.
Additional Notes
According to the patch creator:
This is due to the fact that in generated package-lock.json file, dependencies are not a top level property/field, rather it is nested inside packages i.e each package. As a result, packageLock.dependencies is undefined. This needs some more investigation so I am hoping to raise a separate PR for that
Given that I can't reproduce the original issue (duplication of packages), I would suggest that @desrosj does a double check to see what is going on. Also, it would be great to get additional information and thoughts from jtquip88
, as I'm not really sure how is affecting both the error and the patched version.
I think that if we just wanted to wipe the error, something as minimal as this:
const versionConflicts = Object.entries(packageLock.dependencies || {});
Would have done the trick as well.
PS: It's a pain to test this, it takes one year to run all the processes.
Supplemental Artifacts
This is the full error:
TypeError: Cannot convert undefined or null to object at Function.entries (<anonymous>) at getMismatchedNonWordPressDependencies (/home/admin/src/wordpress/wordpress-develop/tools/release/sync-gutenberg-packages.js:110:34) at main (/home/admin/src/wordpress/wordpress-develop/tools/release/sync-gutenberg-packages.js:36:28) at Object.<anonymous> (/home/admin/src/wordpress/wordpress-develop/tools/release/sync-gutenberg-packages.js:225:1) at Module._compile (node:internal/modules/cjs/loader:1469:14) at Module._extensions..js (node:internal/modules/cjs/loader:1548:10) at Module.load (node:internal/modules/cjs/loader:1288:32) at Module._load (node:internal/modules/cjs/loader:1104:12) at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:174:12) at node:internal/main/run_main_module:28:49
@SirLouen commented on PR #8431:
3 months ago
#6
@jtquip88 please check this
#7
follow-up:
↓ 8
@
3 months ago
@SirLouen Thanks for testing this.
Since there were no duplicates to begin with, it's expected that this patch wouldn’t cause any differences. The patch doesn’t materially change how we fetch and structure dependencies—it simply ensures that local .npmrc settings are honored. This means that if there were duplicates, they would now be deduplicated due to the prefer-dedupe=true setting in .npmrc.
Regarding this error:
TypeError: Cannot convert undefined or null to object:
I didn’t include a fix for this because I assumed we’d want to investigate and address it separately. This error occurs because the parsing logic expects "dependencies" to be a top-level property in package-lock.json. However, in newer versions of npm (v7+), dependencies are nested under individual packages within the "packages" section.
Until npm v6, the "dependencies" field was indeed a top-level property. The parsing logic was likely written when npm v6 was being used so this means the issue has existed for a while but remained unnoticed.
Next steps:
If we still want to keep the version conflict resolution logic, we could rewrite it to support the new structure in package-lock.json. I’m happy to take that on—just wanted to align with the team here first If we want to fix it or just suppress the error.
cc @desrosj
#8
in reply to:
↑ 7
@
2 months ago
- Keywords needs-test-info close added; needs-testing-info removed
- Resolution set to invalid
- Status changed from new to closed
I've provided some testing instructions just in case anyone else could replicate this issue that @desrosj is reporting. I couldn't be on my side, and we are assuming there are no duplicates to begin with.
Replying to jtquip88:
I didn’t include a fix for this because I assumed we’d want to investigate and address it separately. This error occurs because the parsing logic expects "dependencies" to be a top-level property in package-lock.json. However, in newer versions of npm (v7+), dependencies are nested under individual packages within the "packages" section.
In this specific regard, I agree with you, and I would suggest you opening a new ticket with the proposed changes to reanimate the conversation around this topic and if possible adding some testing instructions to replicate.
I'm going to mark this post as a close
candidate with a potential worksforme
.
#9
@
2 months ago
- Resolution invalid deleted
- Status changed from closed to reopened
I accidentally closed. Reopening
#10
@
5 days ago
- Keywords has-test-info added; reporter-feedback needs-test-info close removed
Thanks for the PR @jtquip88.
The PR looks like it should fix the issue. But I'm having trouble confirming because I'm still seeing duplicates.
For anyone looking to test, this expands on my originally listed steps and is probably the best process to follow outside of actually using the script to apply updates:
- Create a branch. This can be from
trunk
or a recent version like6.7
(I don't think6.8
will have any updates currently). - Run
npm dedupe
(--verbose
will give you more detail, but is not required). - Commit any changes that result. You now have a clean, deduped lock file.
- Run
npm run sync-gutenberg-packages
. If you used a branch (say6.7
), then you will need to include-- --dist-tag=wp-<VERSION>
whereVERSION
represents the major branch you are attempting to update (more documentation on the script itself can be found in the block editor handbook). - Run
npm run post-sync-gutenberg-packages
. - Commit any changes to your branch. You now have an updated branch containing the latest changes for that WP major version.
- Run
npm dedupe
again.
Before the patch, this should result in new changes to the package lock file. This demonstrates that the prefer-dedupe
option in the .npmrc
file is not respected.
I am currently unable to reproduce the issue when using trunk
to test without the PR, even though there are package updates that are applied.
However, I was able to reproduce when using the 6.7
branch and npm run sync-gutenberg-packages -- --dist-tag=wp-6.8
(note updating to the next major branch) and the PR did not fix the problem when rerunning.
A note about what duplicates means in this context to clarify. Running npm dedupe
attempts to reduce duplication within the package tree stored within the package-lock.json
file. So it's not that packages will be listed twice in package.json
, but rather once (or as few times as possible) within package-lock.json
. These changes will be really hard to notice on manual review because the lock file is over 37K lines. The best way to notice any changes would be running git diff
.
#11
@
5 days ago
Thanks for the detailed testing steps @desrosj. I am going to try to test this again with 6.7
branch and update here.
#12
@
5 days ago
- Keywords changes-requested added
Reproduction Report
Description
✅ This report validates that the issue can be reproduced.
Environment
- WordPress: 6.7-src
- PHP: 8.2.28
- Server: nginx/1.29.0
- Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
- Browser: Chrome 138.0.0.0
- OS: Windows 10/11
- Theme: Twenty Twenty-Five 1.2
- MU Plugins: None activated
- Plugins:
- Test Reports 1.2.0
Actual Results
- ✅ Error condition occurs
Additional Notes
Followed the steps with 6.7 branch
During npm run sync-gutenberg-packages -- --dist-tag=wp-6.8
the error is still there as expected
tools/release/sync-gutenberg-packages.js:110 const versionConflicts = Object.entries( packageLock.dependencies ) TypeError: Cannot convert undefined or null to object at Function.entries (<anonymous>) at getMismatchedNonWordPressDependencies (/home/user/src/wordpress/wordpress-develop/tools/release/sync-gutenberg-packages.js:110:34) at main (/home/user/src/wordpress/wordpress-develop/tools/release/sync-gutenberg-packages.js:36:28) at Object.<anonymous> (/home/user/src/wordpress/wordpress-develop/tools/release/sync-gutenberg-packages.js:225:1) at Module._compile (node:internal/modules/cjs/loader:1469:14) at Module._extensions..js (node:internal/modules/cjs/loader:1548:10) at Module.load (node:internal/modules/cjs/loader:1288:32) at Module._load (node:internal/modules/cjs/loader:1104:12) at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:174:12) at node:internal/main/run_main_module:28:49
But what has changed is that the execution ended in a warning I can't remember in my previous run:
Running "verify:source-maps" task Warning: The build/wp-includes/js/dist/data.js file must not contain a sourceMappingURL. Use --force to continue. Aborted due to warnings
So I run with npm run sync-gutenberg-packages --force -- --dist-tag=wp-6.8
and still fails with the same warning 🤔 (I also removed the build/ dir before doing this to double check if there could be a conflict for some reason)
Anyway I ignored this, and I went for the postsync
and finally again the dedupe
I can confirm there are dupes but only 2:
- pify
- make-dir
In fact, in the first dedupe run, I can't really see any dupes.
Maybe the problem is specifically related to those two packages that are somehow messing around? @desrosj have you found more dupes that these two?
npm install in sync-gutenberg-packages script is nto respecting settings specified in .npmrc file. To troubleshoot, I ran
npm config list
from inside the sync-gutenberg-packages script and observed the following output.This indicates .npmrc settings are being overridden by environment variables. This means the beahvior might differ based o where script is ran. Made a change to explicitly use local .npmrc file while issuing
npm install
command.Trac ticket: [](https://core.trac.wordpress.org/ticket/62839)