Skip to content

fix(arborist): node.target can be null - #7065

Closed
ljharb wants to merge 1 commit into
npm:latestfrom
ljharb:arborist-no-target
Closed

fix(arborist): node.target can be null#7065
ljharb wants to merge 1 commit into
npm:latestfrom
ljharb:arborist-no-target

Conversation

@ljharb

@ljharbljharb commented Dec 8, 2023

Copy link
Copy Markdown
Contributor

@ljharb
ljharb requested a review from a team as a code ownerDecember 8, 2023 23:15
@ljharbljharb added Bug thing that needs fixing Release 10.x labels Dec 8, 2023
@ljharb

ljharb commented Dec 8, 2023

Copy link
Copy Markdown
ContributorAuthor

a node 16 test failed but since node 16 isn't supported anymore, i assume those tests can be dropped (and node 21 added)?

@ljharb
ljharbforce-pushed the arborist-no-target branch 2 times, most recently from ba5978b to ba53945CompareJanuary 18, 2024 22:54
@wraithgarwraithgar added the pr: needs tests requires tests before merging label Jan 19, 2024
@lukekarryslukekarrys self-assigned this Jan 29, 2024
@lukekarrys

Copy link
Copy Markdown
Contributor

I am still planning to add a failing test case for this in smoke-tests, but I do think this fix looks good.

@ljharb

Copy link
Copy Markdown
ContributorAuthor

Thanks! I'm looking forward to it landing and being unblocked, and if there's anything concrete I can do to hurry it along I'd love pointers.

@dmnsgn

dmnsgn commented Feb 21, 2024

Copy link
Copy Markdown

I am getting a related Cannot destructure property 'package' of 'node.target' as it is null. but coming from another point in arborist's code than the above fix:

const{package: pkg, hasInstallScript }=node.target

Using arborist@7.3.1 in node@v21.6.2 for a package with some dependencies as file: symlinks (with some being inter-dependent).

Stacktrace:

at [addToBuildSet] (/Users/myname/my-package/node_modules/@npmcli/arborist/lib/arborist/rebuild.js:270:22)at [buildQueues] (/Users/myname/my-package/node_modules/@npmcli/arborist/lib/arborist/rebuild.js:210:41)at [build] (/Users/myname/my-package/node_modules/@npmcli/arborist/lib/arborist/rebuild.js:182:29)at Arborist.rebuild (/Users/myname/my-package/node_modules/@npmcli/arborist/lib/arborist/rebuild.js:100:25)at async [reifyPackages] (/Users/myname/my-package/node_modules/@npmcli/arborist/lib/arborist/reify.js:252:11)at async Arborist.reify (/Users/myname/my-package/node_modules/@npmcli/arborist/lib/arborist/reify.js:171:5)

@lukekarrys

Copy link
Copy Markdown
Contributor

@dmnsgn That looks like a different bug. Can you open a new issue about it including your package.json with the interdependent symlinks?

@dmnsgn

Copy link
Copy Markdown

@dmnsgn That looks like a different bug. Can you open a new issue about it including your package.json with the interdependent symlinks?

Sorry, I am having a hard time making a reduced case but a clue for solving the issue is removing a leftover package-lock.json in a workspace.
Let me just add that I was also getting Cannot read properties of undefined (reading 'target') when running npm query '*:missing' and the following stack (workspace.target being undefined at execWorkspaces):

npm info using npm@10.2.4npm info using node@v21.6.2// ...npm verb stack TypeError: Cannot read properties of undefined (reading 'target')npm verb stack at Query.execWorkspaces (/usr/local/lib/node_modules/npm/lib/commands/query.js:104:33)npm verb stack at async module.exports (/usr/local/lib/node_modules/npm/lib/cli-entry.js:61:5)

Not sure it is worth opening an issue if I can't provide repro easily but maybe the above might help someone or "leftover package-lock" might ring a bell to someone who's worked on this.

@dmnsgn

Copy link
Copy Markdown

Another clue is: one of the workspace was dependent on another workspace but its dependency version was different.

Example: a is dependent on b@0.0.1, but b package.json version is 0.0.2.

.
+-- package.json
`-- packages
+-- a@0.0.1
| `-- package.json (depends on b@0.0.1)
`-- b@0.0.2
`-- package.json

That solves it for me so I'll stop here and open another issue if I encounter these errors again. Hope that helps someone.

@AmirSa12

AmirSa12 commented May 30, 2024

Copy link
Copy Markdown
Contributor

I am getting an error about the target being null but this PR did not fix it

wraithgar pushed a commit that referenced this pull request Jul 1, 2024
…#7579)
<!-- What / Why -->
If a node represents a symbolic link or a file dep (node.isLink is
true), its target is expected to reference another node in the
dependency tree. If the linking is not done correctly or is incomplete,
node.target might be null.
<!-- Describe the request in detail. What it does and why it's being
changed. -->
in this PR, a null check is added to ensure node.target is not null or
before proceeding, which will prevent causing errors like:
`npm error Cannot set properties of null (setting 'peer')` ## References
Related to #7065, Fixes#6622, #5007,
Closes#6622, #5007
@owlstronautowlstronaut self-assigned this Apr 15, 2025
@owlstronaut

Copy link
Copy Markdown

Looks like this was resolved here #7579

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bugthing that needs fixingpr: needs testsrequires tests before mergingRelease 10.x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] npm install fails in my project since v7

6 participants

@ljharb@lukekarrys@dmnsgn@AmirSa12@owlstronaut@wraithgar