[BUG] specific diff3 conflict is not parsed

sersorrel commented 12 months ago

Is there an existing issue for this?

[X] I have searched the existing issues

This issue exists in the latest npm version

[X] I am using the latest npm

Current Behavior

When package-lock.json is conflicted, I believe npm has some functionality to automatically resolve the conflicts when you npm install. However, this doesn't work consistently when git is configured to use diff3 conflict markers – here I get a "must provide string spec" error, much like #2117:

$ npm i
npm ERR! must provide string spec

npm ERR! A complete log of this run can be found in: /Users/ah37/.npm/_logs/2023-11-13T16_41_08_559Z-debug-0.log

That log contains the following stack trace:

35 verbose stack TypeError: must provide string spec
35 verbose stack     at new Edge (/nix/store/fwgfw6i5q1hv49bgfl96bmzv72l98khy-nodejs-18.18.2/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/edge.js:68:13)
35 verbose stack     at #loadDepType (/nix/store/fwgfw6i5q1hv49bgfl96bmzv72l98khy-nodejs-18.18.2/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/node.js:896:9)
35 verbose stack     at [Arborist.Node._loadDeps] (/nix/store/fwgfw6i5q1hv49bgfl96bmzv72l98khy-nodejs-18.18.2/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/node.js:874:22)
35 verbose stack     at new Node (/nix/store/fwgfw6i5q1hv49bgfl96bmzv72l98khy-nodejs-18.18.2/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/node.js:260:20)
35 verbose stack     at #loadNode (/nix/store/fwgfw6i5q1hv49bgfl96bmzv72l98khy-nodejs-18.18.2/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/load-virtual.js:261:18)
35 verbose stack     at #resolveNodes (/nix/store/fwgfw6i5q1hv49bgfl96bmzv72l98khy-nodejs-18.18.2/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/load-virtual.js:190:43)
35 verbose stack     at #loadFromShrinkwrap (/nix/store/fwgfw6i5q1hv49bgfl96bmzv72l98khy-nodejs-18.18.2/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/load-virtual.js:90:48)
35 verbose stack     at Arborist.loadVirtual (/nix/store/fwgfw6i5q1hv49bgfl96bmzv72l98khy-nodejs-18.18.2/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/load-virtual.js:63:35)
35 verbose stack     at async Arborist.buildIdealTree (/nix/store/fwgfw6i5q1hv49bgfl96bmzv72l98khy-nodejs-18.18.2/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js:193:7)
35 verbose stack     at async Promise.all (index 1)

Expected Behavior

I'd hoped that npm would be able to understand both regular conflict markers and the (more useful) diff3 markers.

The error can, however, be worked around via git checkout --conflict=merge package-lock.json.

Steps To Reproduce

Get into a state where your package-lock.json contains a merge conflict
Run git checkout --conflict=diff3 package-lock.json to make sure it uses diff3 markers
Run npm i
See "must provide string spec" error

Environment

npm: 9.8.1
Node.js: 18.18.2
OS Name: macOS Ventura 13.6.1
System Model Name: MacBook Pro

npm config:

; node bin location = /nix/store/fwgfw6i5q1hv49bgfl96bmzv72l98khy-nodejs-18.18.2/bin/node
; node version = v18.18.2
; npm local prefix = /Users/ah37
; npm version = 9.8.1
; cwd = /Users/ah37
; HOME = /Users/ah37
; Run `npm config ls -l` to show all defaults.

sersorrel commented 12 months ago

Note: this issue is also present in npm 10.1.0.

wraithgar commented 12 months ago

It does have some functionality but it's a best effort. The resolution is done by https://github.com/npm/parse-conflict-json.

Since it has never done diff3 I'm hesitant to call this a bug. Adding diff3 would potentially mean adding that capability to the underlying https://www.npmjs.com/package/just-diff module, and it definitely would mean adding diff3 support to parse-conflict-json.

We'd welcome PRs that added this, but it'd be a new feature. This feature is not something the npm team would be actively working on though.

sersorrel commented 12 months ago

parse-conflict-json looks like it's meant to have support for diff3 diffs, though – the tests include a file with diff3 markers, and it includes a regex to match the ||||||| marker you find in diff3 diffs but not regular diffs.

sersorrel commented 12 months ago

Here's a gist of the package.json and package-lock.json I experienced this with.

wraithgar commented 12 months ago

Ah ok, I just searched for 'diff3' not the markers themselves. It looks like diff3 is supported but your particular conflict isn't.

npm / cli