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

repository validation fails in some cases #116

Open
Andarist opened this issue Sep 29, 2021 · 8 comments
Open

repository validation fails in some cases #116

Andarist opened this issue Sep 29, 2021 · 8 comments

Comments

@Andarist
Copy link
Collaborator

Andarist commented Sep 29, 2021

For this:

{ "rootRepositoryField": "https://github.com/Thinkmill/manypkg" }

we get this back from parse-github-url:

{
  "protocol": "https:",
  "slashes": true,
  "auth": null,
  "host": "github.com",
  "port": null,
  "hostname": "github.com",
  "hash": null,
  "search": null,
  "query": null,
  "pathname": "Thinkmill/manypkg",
  "path": "Thinkmill/manypkg",
  "href": "https://github.com/Thinkmill/manypkg",
  "filepath": null,
  "owner": "Thinkmill",
  "name": "manypkg",
  "repo": "Thinkmill/manypkg",
  "branch": "master",
  "repository": "Thinkmill/manypkg"
}

As we might notice the branch has defaulted to master which in turn makes per-package repository field like https://github.com/Thinkmill/manypkg/tree/main/packages/cli incorrect and results in the error:

☔️ error @manypkg/cli has a repository field of "https://github.com/Thinkmill/manypkg/tree/main/packages/cli" when it should be "https://github.com/Thinkmill/manypkg/tree/master/packages/cli"

The quickest fix for me was to just remove the root "repository" field but I think that the situation here should be improved so nobody would face that in the future.

@emmatown
Copy link
Member

We don't actually look at the branch from parse-github-url, you can set the branch used for the rule in the root package.json:

"manypkg": {
  "defaultBranch": "main"
}

@Andarist
Copy link
Collaborator Author

Hah, right! I've just checked that in here we get incorrect~ (in my situation) branch and I've assumed that it's the culprit.

Since most of the stuff is really convention-based and not configuration-based. Why this option does exist? Why it was not chosen to validate and fix the rootRepositoryField so it would contain a branch name explicitly and for that to be the source of this "option"? In addition to that - without rootRepositoryField the per-package packag.json#repository is not validated at all so each package could end up with highly different/broken URLs there

@emmatown
Copy link
Member

emmatown commented Oct 14, 2021

What would the root repository field look like with the branch name included? afaik, there isn't a form that would include it?

@Andarist
Copy link
Collaborator Author

If we visit https://github.com/Thinkmill/manypkg and press Y on the keyboard we are being redirected to an URL containing the hash (eg. https://github.com/Thinkmill/manypkg/tree/5f6cdedf6843d60144c1ea65b5a8ef0c4b7f0bd5) - the URL isn't really bound to a hash though, it can take any git revision. So we can just replace this with a branch name and end up with a valid URL such as: https://github.com/Thinkmill/manypkg/tree/master

@emmatown
Copy link
Member

I did not know that was a thing! Supporting that would be great

@Andarist
Copy link
Collaborator Author

Can't say it's anywhere on my todo list but it's great to keep this as an issue "for the better times" 😉

@Andarist
Copy link
Collaborator Author

TIL that repository.directory is supported:

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

https://docs.npmjs.com/cli/v6/configuring-npm/package-json#repository

@emmatown
Copy link
Member

Yeah though I'm not sure I've seen anything that actually uses it, e.g. the repository link for https://www.npmjs.com/package/react goes to the root of the repo which is quite annoying but the link on https://www.npmjs.com/package/@emotion/react goes to the package directory

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

No branches or pull requests

2 participants