Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect unpublished packages #2

Closed
wants to merge 1 commit into from
Closed

Detect unpublished packages #2

wants to merge 1 commit into from

Conversation

bokub
Copy link
Contributor

@bokub bokub commented Aug 28, 2018

Fixes #1

I couldn't unpublish 'sunglasses' 😎 , but I found 'leopard' 🐆 to be unpublished too so I added it to the tests

@sholladay
Copy link
Owner

sholladay commented Aug 28, 2018

Thanks for putting this together! 👏

I wonder what meta will look like for a package where a specific version was unpublished, rather than every version. The latter is npm's default, but you can specify a version in npm unpublish.

As for how to find a sample, perhaps left-pad still has older versions that are unpublished and were never restored.

@bokub
Copy link
Contributor Author

bokub commented Aug 28, 2018

I've just tested, and here is the result:
(I've unpublished the 0.0.1)

{
  // ...
  'dist-tags': { latest: '0.0.2' },
  versions:
   { '0.0.2':
      {
      //...
      }
   },
  time:
   { created: '2018-08-28T17:03:13.524Z',
     '0.0.1': '2018-08-28T17:03:13.692Z',
     modified: '2018-08-28T17:07:04.066Z',
     '0.0.2': '2018-08-28T17:06:04.272Z' },

@sholladay
Copy link
Owner

Awesome, that's about what I would have expected.

The reason that matters is that squatter takes a second (currently undocumented) version argument. I'm not sure if there is a strong use case for it, but I implemented it so that i could use it in the tests if it turned out that the integration tests were too flakey, since they use the latest of real packages and someone could publish a new version that breaks those tests at any time. So far that has never been a problem, but I'm keeping an eye on it.

Anyway, in order to handle the version argument correctly, I guess unpublished() would have to first resolve the version using dist-tags (but handle it not existing) and then check if that version exists in Object.keys(meta.versions). Based on the output you posted in the issue, it seems like when all versions are unpublished, they move meta.versions to meta.time.unpublushed.versions. So I suppose it would have to handle that, too.

Or to simplify everything, we could get rid of the version argument.

@bokub
Copy link
Contributor Author

bokub commented Aug 29, 2018

Hmm, I 'm not sure to get it 😕

Being unpublished cannot depend on a version: a package is either completely unpublished (so not a squatter), either has one or more published versions, in that case the additional tests need to be run.

By the way, I really don't understand how npm works:
I've unpublished all the versions of my imaginary package (I don't remember the order tho), and here is what the meta looks like:

{
  // ...
  'dist-tags': { latest: '0.0.2' },
  versions: { 
    '0.0.2': {
    // ...
    }
  },
  time:
   { created: '2018-08-28T17:03:13.524Z',
     '0.0.1': '2018-08-28T17:03:13.692Z',
     modified: '2018-08-28T19:34:12.799Z',
     '0.0.2': '2018-08-28T17:06:04.272Z',
     '0.0.3': '2018-08-28T17:19:48.544Z',
     '0.0.4': '2018-08-28T17:22:03.420Z',
     '0.1.0': '2018-08-28T19:21:15.659Z' },
  }

I really don't get it. The npmjs.com page shows a 404, but the unpkg.com site still has the 0.0.2 version (and only this one).

Note: It may change in a few hours, because the npm command line says I should wait 24 hours after a package is completely unpublished to publish again.

@sholladay
Copy link
Owner

As the owner, you can choose to unpublish all versions or only some versions. The default behavior when you run npm unpublish is to unpublish all versions, but people can choose to unpublish specific versions if they feel like it (e.g. 0.0.2 had a major security flaw, but 0.0.3 has the fix, and you want to unpublish only 0.0.2 to ensure no one is affected by the security flaw).

So unpublishing depends upon versioning in the same way that publishing does. The only part I'm not super familiar with is how they represent this in the registry data. I'll help investigate that and see if there is a way for us to tell that a specific version was unpublished or whether the entire namespace was unpublished. Give me a few days and I'll look into it!

@brekk
Copy link

brekk commented Nov 25, 2018

Any follow-up on this? I recently was using npm-name-cli which uses squatter as a dependency -- I was looking to publish zark but as far as I can tell from the registry it is unpublished / squatted upon by the user ~zark:

npm-name zark
⠇ Checking name on npmjs.com…

Sat and hung for a while, until I hit Ctrl-C and then I saw:

^CTypeError: Cannot read property 'latest' of undefined
    at versionPkg (/[...]/.config/yarn/global/node_modules/squatter/lib/get-package.js:9:36)
    at squatter ([...[/.config/yarn/global/node_modules/squatter/index.js:42:17)
    at process._tickCallback (internal/process/next_tick.js:68:

I'd love to be able to use npm-name / squatter to track this status.

@sholladay
Copy link
Owner

I will be fixing this, for sure. Most likely merging this PR as-is. I just need to find the time to verify my assumptions about the registry data are correct. Tentatively thinking I should be able to do that middle of this week. Soon as this is done, I will make a patch release. Thanks for your patience. :)

@moritzruth
Copy link

any updates on this? @sholladay

@@ -1,6 +1,6 @@
{
"name": "squatter",
"version": "0.2.0",
"version": "0.2.1",
Copy link
Owner

Choose a reason for hiding this comment

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

Could you please either undo this change or give me write access to your fork so I can do it for you? (I'll bump the version during the release process.)

@sholladay
Copy link
Owner

Hi! I finally got around to this. Ready to merge with that one tweak. Thanks for your patience. :)

@sindresorhus
Copy link

@bokub Ping

@bokub
Copy link
Contributor Author

bokub commented Jul 1, 2019

Hi Sindre
I can't edit my PR because I've deleted the fork a long time ago 🙃
Maybe @sholladay can make an edit before or after merging?

If not, I'll make a new PR tomorrow when I'm around a computer

@sholladay
Copy link
Owner

For some reason, GitHub will allow me to merge, but not edit. I presume this is because the fork was deleted.

Don't worry about recreating the fork. What I'll do is squash merge and then amend the commit on master.

Thanks for your work on this!

@bokub bokub mentioned this pull request Jul 2, 2019
@bokub
Copy link
Contributor Author

bokub commented Jul 2, 2019

I've duplicated this PR in #5 😊

@bokub bokub closed this Jul 2, 2019
@bokub
Copy link
Contributor Author

bokub commented Jul 22, 2019

@sholladay Ping 🙋

@sholladay
Copy link
Owner

I merged PR #5. Thanks for your work. I am aiming to do a release next weekend, possibly sooner.

@sindresorhus
Copy link

@sholladay Reminder about releasing.

@sholladay
Copy link
Owner

This fix has been released. Thanks for your patience, y'all. I've been taking care of my father who is very sick. I'm behind on many things, but OSS keeps me going. :)

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

Successfully merging this pull request may close these issues.

versionPkg() throws "Cannot read property 'latest' of undefined"
5 participants