workshopper / how-to-npm

A module to teach you how to module.
https://www.npmjs.com/package/how-to-npm
ISC License
1.12k stars 221 forks source link

registry path for scoped package tarball should include scope subdirectory under dash directory #127

Closed vobarian closed 7 years ago

vobarian commented 7 years ago

The JSON sent by npm publish contains the property versions.XXX.dist.tarball which contains the scope name twice for a scoped package; for example: "http://localhost:15443/@chad/myhowtonpm/-/@chad/myhowtonpm-1.0.0.tgz". The mock registry server attempts to remove the second occurrence of @scope from the file path, which causes an error if users try to create another package into which they install the package created in the workshopper using the mock server. The path should be left as-is to match the tarball property in body.json so npm can GET it. Removing this code has the additional benefit of fixing a bug on Windows because the regex does not consider backslashes in the path.

Fixes: https://github.com/workshopper/how-to-npm/issues/122 Fixes: https://github.com/nodeschool/discussions/issues/1561 Fixes: https://github.com/nodeschool/discussions/issues/2049

Please note this would supersede PR https://github.com/workshopper/how-to-npm/pull/68

Additional Explanation

Given a package @chad/myhowtonpm, running npm publish with npm 5.0.3 will PUT JSON something like this:

{
    "_id": "@chad/myhowtonpm",
    "name": "@chad/myhowtonpm",
    "description": "This is my package.",
    "dist-tags": {
        "latest": "1.0.0"
    },
    "versions": {
        "1.0.0": {
            "name": "@chad/myhowtonpm",
            "version": "1.0.0",
            "main": "index.js",
            "scripts": {
                "test": "node test.js"
            },
            "author": "",
            "license": "ISC",
            "description": "This is my package.",
            "dependencies": {
                "@linclark/pkg": "^1.0.2"
            },
            "repository": {
                "type": "git",
                "url": "http://www.vobarian.com/"
            },
            "readme": "This is my package. \r\n",
            "readmeFilename": "README.md",
            "_id": "@chad/myhowtonpm@1.0.0",
            "_npmVersion": "5.0.3",
            "_nodeVersion": "8.0.0",
            "_npmUser": {
                "name": "chad",
                "email": "test@example.com"
            },
            "maintainers": [
                {
                    "name": "chad",
                    "email": "test@example.com"
                }
            ],
            "dist": {
                "integrity": "sha512-8H6LDqLEk9DR3DENJh11GilaxWPwheCuxKXCiuTDESMZ4ilduTPDa4kFD2sP0iL2r7refGg0FQQupa58et0CEQ==",
                "shasum": "fc9a1b352764777a8333ef77ad2f7cc72c989c9e",
                "tarball": "http://localhost:15443/@chad/myhowtonpm/-/@chad/myhowtonpm-1.0.0.tgz"
            }
        }
    },
    "readme": "This is my package. \r\n",
    "maintainers": [
        {
            "name": "chad",
            "email": "test@example.com"
        }
    ],
    "_attachments": {
        "@chad/myhowtonpm-1.0.0.tgz": {
            "content_type": "application/octet-stream",
            "data": "...",
            "length": 668
        }
    }
}

Note that the tarball property contains two subdirectories following the pattern @scope. In order for a user to be able to npm install the package the file has to be found at this location. Hence the current code which attempts to strip the second occurrence of @scope is not correct; in fact I have verified that attempting to install the package into a second package causes an error because of this.

With the new code, the variables trace out as follows:

dir = "registry/@chad/myhowtonpm"
tgzBase = "@chad/myhowtonpm-1.0.0.tgz"
tgzFile = "registry/@chad/myhowtonpm/-/@chad/myhowtonpm-1.0.0.tgz"
path.dirname(tgzFile) = "registry/@chad/myhowtonpm/-/@chad"

The path tgzFile matches the tarball property and the correct directory is created in the mkdirp call. It is possible to install the package created in the workshopper into a second package later on (assuming the user copies .npmrc to use the mock server). Also, since the regex is no longer needed, the bug that breaks publishing on Windows is simply gone.

watilde commented 7 years ago

Thanks for the patch! As you said the patch including the scope name should be resolved by the path module. Will do test on my local and ship it.

watilde commented 7 years ago

This patch was released as how-to-npm@2.4.3 🎉

Thanks again for putting this together.