feat: `npm repo` support `repository.directory` field #163

ybiquitous · 2019-02-18T15:46:14Z

Summary

This PR makes npm repo possible to open a given repository's full URL instead of its root URL with the new repository.directory field.

For example, npm repo react-dom opens https://github.com/facebook/react/tree/master/packages/react-dom.

Background

npm@6.8.0 shipped the new feature to support repository.field in package.json.

For example:

{
  "repository": {
    "type": "git",
    "url": "https://github.com/facebook/react.git",
    "directory": "packages/react-dom"
  }
}

See also #140

ybiquitous · 2019-02-18T15:51:56Z

I think this change should test the following cases:

npm repo <github_hosted_package>
npm repo <gitlab_hosted_package>
npm repo <bitbucket_hosted_package>
(and so on...?)

But sadly I didn't know how to test these cases. 😢
To test the cases, should I change npm-registry-mock package, or is there some better way?

cli/test/tap/repo.js

Line 2 in 86f5bdb

var mr = require('npm-registry-mock')

I hope someone would help me about testing... 🙏

toyokazu03102478

お願いいたします

zkat · 2019-02-19T18:04:29Z

Here's an example of how to mock a packument request: https://github.com/npm/cli/blob/release-next/test/tap/aliases.js#L29-L54

You don't need to bother with mocking the tarball.

ybiquitous · 2019-02-21T08:06:45Z

@zkat Thanks for your advice! I'll rebase after #167 is merged.

See #140

ybiquitous · 2019-03-21T04:11:48Z

test/tap/repo.js

+    t.comment(stderr)
+
+    const res = fs.readFileSync(outFile, 'ascii')
+    t.equal(res, 'https://github.com/foo/test-repo-with-directory/tree/master/some%2Fdirectory\n')


I expected .../some/directory, not .../some%2Fdirectory. 🤔
But I find this is a problem of hosted-git-info package.
So, I will open a new PR on npm/hosted-git-info. 💪

The relevant code is here:
https://github.com/npm/hosted-git-info/blob/067fd7f3559fc051fd56528f6231a70ec4e634d0/git-host.js#L38-L40

This has been fixed! So this test will actually fail now. We'll fix this up when landing this PR! :D

@mikemimik Thanks for your response. I have not touched this PR for a long time, but can I start again?

If you have any advice, I would be glad to accept!

It's not necessary to start again. The @npm/cli-team triaged this pull-request today and we're accepting it into 6.14.0. We can fix up the test when we pull it into the release, it's no worries :)

You noted that hosted-git-info does some weird url-encoding, and you had to use "%2f". Since then hosted-git-info has had some changes and it's fixed. It now returns properly. The change to the test is just changing %2f back to / :D

I understand it. I'm looking forward to a new release 😊

ybiquitous · 2019-03-21T04:40:40Z

I added a test case and CI passed!

Notes

I thought test cases for possible Git hostings were necessary (not only GitHub).
But hosted-git-info package already tests possible hostings, so I added just the GitHub case and I think it enough to verify this PR feature.

ybiquitous · 2019-04-15T02:07:20Z

@zkat I added the tests, so is there something I should do to review this PR?

@ybiquitous

See #140 PR-URL: #163 Credit: @ybiquitous Close: #163 Reviewed-by: @mikemimik

This is the standard configuration value for deep-linking into a package's directory within a monorepo. npm RFC: https://github.com/npm/rfcs/blob/latest/implemented/0010-monorepo-subdirectory-declaration.md This will ensure the packages' pages on npmjs.org will link to the correct directory. Also, _eventually_, it will ensure the `npm repo` command opens to the correct location: npm/cli#163 Lastly, this is _required_ for monorepo packages to be successfully published to the GitHub Package Registry (if that is ever considered in the future). https://help.github.com/en/packages/using-github-packages-with-your-projects-ecosystem/configuring-npm-for-use-with-github-packages#publishing-multiple-packages-to-the-same-repository

Summary: Configure each package's package.json#repository.directory property to properly reflect the location of the package within the repository. This is the standard configuration value for deep-linking into a package's directory within a monorepo. npm RFC: https://github.com/npm/rfcs/blob/latest/implemented/0010-monorepo-subdirectory-declaration.md This will ensure the packages' pages on npmjs.org will link to the correct directory. Also, _eventually_, it will ensure the npm repo command opens to the correct location: npm/cli#163 Lastly, this is _required_ for monorepo packages to be successfully published to the GitHub Package Registry (if that is ever considered in the future). https://help.github.com/en/packages/using-github-packages-with-your-projects-ecosystem/configuring-npm-for-use-with-github-packages#publishing-multiple-packages-to-the-same-repository

@ybiquitous

Related: #140 PR-URL: #163 Credit: @ybiquitous Close: #163 Reviewd-By: @isaacs

isaacs · 2020-08-04T08:12:45Z

This will be in the v7 beta. Thanks!

toyokazu03102478 approved these changes Feb 18, 2019

View reviewed changes

ybiquitous marked this pull request as ready for review February 19, 2019 00:19

ybiquitous requested a review from a team as a code owner February 19, 2019 00:19

zkat added semver:minor new backwards-compatible feature in-progress labels Feb 19, 2019

feat: npm repo support repository.directory field

0bf3299

See #140

ybiquitous commented Mar 21, 2019

View reviewed changes

ybiquitous mentioned this pull request Mar 21, 2019

fix(git-host): disallow URI-encoded slash (%2F) in path npm/hosted-git-info#44

Closed

isaacs mentioned this pull request Jul 1, 2019

Release: npm@6.10.0 #205

Merged

snyk-bot mentioned this pull request Aug 13, 2019

[Snyk] Upgrade npm from 6.6.0 to 6.10.2 devopsred/gaia#4

Open

settings bot removed in-progress labels Nov 5, 2019

mikemimik added the Release 6.x work is associated with a specific npm 6 release label Nov 26, 2019

mikemimik added this to the Release 6.14.0 milestone Nov 26, 2019

mikemimik pushed a commit that referenced this pull request Dec 2, 2019

feat: npm repo support repository.directory field

980200b

See #140 PR-URL: #163 Credit: @ybiquitous Close: #163 Reviewed-by: @mikemimik

mikemimik added Priority Backlog a "backlogged" item that will be tracked in a Project Board pr: needs tests requires tests before merging labels Dec 3, 2019

mikemimik removed this from the Release 6.14.0 milestone Dec 3, 2019

This was referenced Feb 1, 2020

[Snyk] Fix for 1 vulnerabilities Bhanditz/cli#3

Open

[Snyk] Fix for 1 vulnerabilities bumplzz69/cli#2

Open

[Snyk] Fix for 1 vulnerabilities dongseok1985/cli#3

Open

jasonkarns mentioned this pull request May 6, 2020

Configure repository.directory for monorepo packages optimizely/javascript-sdk#471

Merged

darcyclarke added this to the OSS - Sprint 8 milestone Jun 11, 2020

darcyclarke changed the base branch from latest to release/v7.0.0-beta July 31, 2020 19:01

Merge branch 'release/v7.0.0-beta' into npm-repo-support-directory

f8d665b

isaacs pushed a commit that referenced this pull request Aug 4, 2020

feat: npm repo support repository.directory field

ebf7ac9

Related: #140 PR-URL: #163 Credit: @ybiquitous Close: #163 Reviewd-By: @isaacs

isaacs closed this Aug 4, 2020

ybiquitous deleted the npm-repo-support-directory branch August 4, 2020 08:13

This was referenced Sep 15, 2021

[Snyk] Fix for 1 vulnerabilities Bhanditz/cli#20

Open

[Snyk] Fix for 1 vulnerabilities Jankyboy/cli-1#35

Open

[Snyk] Fix for 1 vulnerabilities mtdev2/cli#77

Open

[Snyk] Fix for 1 vulnerabilities thebpc/cli#31

Open

Omrisnyk mentioned this pull request Apr 30, 2024

[Snyk] Security upgrade npm from 5.6.0 to 6.10.1 Omrisnyk/hibernate-demos#85

Open

Omrisnyk mentioned this pull request May 24, 2024

[Snyk] Security upgrade npm from 5.6.0 to 6.10.1 Omrisnyk/hibernate-demos#88

Open

This was referenced Oct 21, 2024

[Snyk] Fix for 21 vulnerabilities Graysonbarton/cli-3#24

Open

[Snyk] Fix for 21 vulnerabilities Graysonbarton/cli-3#100

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: `npm repo` support `repository.directory` field #163

feat: `npm repo` support `repository.directory` field #163

ybiquitous commented Feb 18, 2019

ybiquitous commented Feb 18, 2019

toyokazu03102478 left a comment

zkat commented Feb 19, 2019

ybiquitous commented Feb 21, 2019

ybiquitous Mar 21, 2019

mikemimik Nov 26, 2019

ybiquitous Nov 27, 2019

mikemimik Nov 27, 2019

ybiquitous Nov 27, 2019

ybiquitous commented Mar 21, 2019

ybiquitous commented Apr 15, 2019

isaacs commented Aug 4, 2020

feat: npm repo support repository.directory field #163

feat: npm repo support repository.directory field #163

Conversation

ybiquitous commented Feb 18, 2019

Summary

Background

ybiquitous commented Feb 18, 2019

toyokazu03102478 left a comment

Choose a reason for hiding this comment

zkat commented Feb 19, 2019

ybiquitous commented Feb 21, 2019

ybiquitous Mar 21, 2019

Choose a reason for hiding this comment

mikemimik Nov 26, 2019

Choose a reason for hiding this comment

ybiquitous Nov 27, 2019

Choose a reason for hiding this comment

mikemimik Nov 27, 2019

Choose a reason for hiding this comment

ybiquitous Nov 27, 2019

Choose a reason for hiding this comment

ybiquitous commented Mar 21, 2019

Notes

ybiquitous commented Apr 15, 2019

isaacs commented Aug 4, 2020

feat: `npm repo` support `repository.directory` field #163

feat: `npm repo` support `repository.directory` field #163