Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

fix: Prevent crash with nameless package #48

Open
wants to merge 1 commit into
base: latest
Choose a base branch
from

Conversation

kf6kjg
Copy link

@kf6kjg kf6kjg commented May 26, 2020

Not every package.json has a name: while npm warns if a name is not provided, it is not required. In my case I have one that should NOT have a name as it's being used only for dependency management, not package description. I know, it's an edge case.

$ npm -v
6.14.5

The workaround is to simply add a name to the package.

I tripped across this myself and found I wasn't the first one: https://stackoverflow.com/questions/59777226/npm-ci-throws-exception-cannot-read-property-length-of-undefined

While the workaround is to simply add a name to the package, the name field is defined as optional and NPM otherwise only warns about it's lack of existence.

I tripped across this myself and found I wasn't the first one: https://stackoverflow.com/questions/59777226/npm-ci-throws-exception-cannot-read-property-length-of-undefined
@lydell
Copy link

lydell commented Sep 6, 2020

I’ve run into this issue as well.

If you have "private": true in your package.json npm won’t even warn about name and version being missing, so I always thought one could safely omit them for “applications” (i.e. things that aren’t packages that you are going to publish on npm). (As a side note, it feels like more parts of npm wrongly assumes name and version always exist – I’ve seen undefined@undefined in logs sometimes.)

Now to the interesting part. What should _incorrectWorkingDirectory do if name is missing? Does the working directory (wd) count as incorrect or not? (If I understand this PR correctly, _incorrectWorkingDirectory will always return true if name is missing?) I have trouble understanding what the point of _incorrectWorkingDirectory is at all – see #49 for more information.

So I wonder what the ultimate fix needs to be ... making _incorrectWorkingDirectory not crash or getting rid of it altogether?

@kf6kjg
Copy link
Author

kf6kjg commented Oct 13, 2020

While I agree that the larger picture needs to be looked at and possibly cleaned, this was an attempt to make the smallest fix that solves the bug. Thus making it easy to review and merge. Re-evaluating and rebuilding whole logic blocks in open source projects, as much as that's tempting, is something I've not had time for since I started coding full time several years ago. Sadness.

@pi0
Copy link

pi0 commented Oct 16, 2020

Hi there! Actually i came here while investigating an issue that was misleading about source of error (nuxt-contrib/opencollective#61). Meanwhile it absolutely makes sense that consumer of npm-lifecycle avoid this issue by giving a default value to pkg.name (to also avoid undefined@undefined) i think it worth merging this by taking the fact into account that when private field is true, name and version are can be optional.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants