Open mojavelinux opened 4 years ago
I should mention that the README works fine on npmjs.com.
I have downloaded and inspected the tarball and I cannot see anything about these packages that would prevent the README from being displayed. Is there a chance it gets the README from somewhere besides the package?
Is it, by chance, trying to fetch the README from GitLab? If so, it's not going to find it there in this case since the README is generated when the package is created. If the script cannot resolve a remote README, can it be changed to use the README from the package file as a fallback?
@Haroenv Any ideas?
btw, same problem on the classic site:
https://classic.yarnpkg.com/en/package/@antora/asciidoc-loader
It just dawned on me to inspect the browser console. Sure enough, it's looking for a README.md file on GitLab.
This breaks the display of all my packages since I don't use Markdown for my package README files, and I spent a lot of effort to write a script to produce a Markdown README to put in the package just so it would display on yarnpkg.com. And now that has broken again.
I also disagree the README should be pulled directly from repository. The repository's README could be well ahead of the latest stable release. The README on yarnpkg.com should show the content from the most recent stable release, otherwise it's not consistent with what the user is downloading / installing.
Looking even closer, it's not even right README from the repository. The script is looking for the README at the repository root. This means yarnpkg.com doesn't have proper support for packages from monorepos. So I'm not even sure if there's any action I can take to rectify this situation from my end, even if I wanted to.
Looks like the same problem for packages hosted on GitHub:
https://yarnpkg.com/package/asciidoctor
(It's looking for README.md in the repository, but it's not going to find one there).
Thanks for the info, this is something we will look into. What I can see from one of those examples already is that the readme is missing top-level, but is showing on a specific version on registry.npmjs.org
@antora/asciidoc-loader
If we can get the information from the latest
version if missing from main, that would solve those issues already.
For Asciidoctor
readme seems to be completely missing :(
asciidoc
An option I'm seeing is to go to unpkg before going to GitHub / gitlab. Unfortunately in those cases readmeFilename
also seems to be missing, so we'll have to assume it's called README.md
(even though it can also be lowercase, no .md, or .markdown)
I don't understand, how is the readme and readmeFilename set on a specific release, but not on the top-level metadata? Do I even have control over this when publishing? The README.md is there in the tarball.
For the record, I haven't changed anything about the release process for more than a year, and this used to work. So I'm confused how this is all of a sudden breaking.
What command are you running to get that output? (I see, it's curl registry.npmjs.com/packagename)
I'm going to https://registry.npmjs.org/@antares/asciidoc-loader to see that. Unfortunately I haven't yet looked into npm itself how it's getting those values out of the tarball.
I think doing something similar to what's done for the changelog (racing some file names), but to unpkg should give a reasonable approximation for what npm is doing (although their source code is closed, boo)
Are you publishing using npm or Yarn? I wonder if that makes a difference
Are you publishing using npm or Yarn? I wonder if that makes a difference
I'm publishing with Yarn npm via Lerna. It appears that's the culprit for why the field is not being set on the top-level manifest...at least that's my theory.
Thanks for the insight, btw!
That's definitely a common enough use case that we can't leave it broken for everyone. Can you try tweaking this setup for a test package and see if there's a combination of tools which makes the readme disappear? In any case, I think we'll have to put another fallback before the repository via unpkg
I'm going to investigate a bit and see if I can figure out why the readme and readmeFilename don't get populated at the top-level when they are populated in the version itself.
I publish another package with npm directly and it doesn't have this problem, so it could be an issue with Lerna...though I need to observe the order of commands first to know what's going on under the covers.
The fact that these fields aren't set on registry.npmjs.com gives me the impression the problem is in fact with the package, not yarnpkg.com. But let's see what I find.
If you're interested (though I'm not presuming you have time), I have documented in detail how to test local releases of Antora in the releasing guide: https://gitlab.com/antora/antora/-/blob/master/releasing.adoc#testing-a-release
Sorry, I'm having a really busy day now, and while it's part of my job, not the main part I'm focusing on right now, so I won't have the time to immediately resolve this, but should see in the next few days though what fallback works best. I can definitely still review PRs though
I'm reasonably confident to report that the root cause of this issue is an old bug in Lerna that has since been fixed. Lerna used to publish the package without the serialized manifest (see https://github.com/lerna/lerna/issues/1843), and in fact published straight out of the directory (cd /tmp/test-release-from/packages/asciidoc-loader && npm publish
).
Lerna has since been updated to publish using the same logic as npm. I can't be entirely sure until I test it with a real release, but verdaccio seems to be reporting that it's receiving the correct metadata.
The tricky part has been updating Lerna without breaking my releases since, as usual, things have changed.
I finally tracked down where in Lerna the manifest is being sent and I can confirm that both the readme and readmeFilename fields are set when using the latest Lerna. I'm feeling confident this is going to fix the immediate problem with these packages in particular.
As for what happens when these fields are missing, it seems like it would be reasonable to
(...assuming that isn't too hard to do).
One thing I noticed is that readme also is missing on some other packages (e.g. instantsearch.js), which aren't published with lerna also are missing the readme & readmeFilename
The workaround for your type of packages indeed shouldn't be too complex, code would be what we have here: https://github.com/algolia/npm-search/blob/5c83b6e4f568d15d2e38c4e9d77c894df4bbc1ef/src/formatPkg.js#L88, but extracted into a function & fall back to pkg.versions[pkg['list-tags'].latest].readme
& same for readmeFilename
I made another prerelease using the latest version of lerna, which uses libnpmpublish under the covers. While the readme and readmeFilename fields are set on the version, they are still not set at the top level. My guess is that they aren't set because that release was tagged with the dist-tag testing (instead of latest). I'm ready to push out a stable release, but I was hoping to resolve this problem first. Now I realize that we just won't know until I do a latest release. So, that's what I'll do.
I have to be honest, the way the registry manifest gets populated by npm is so unbelievably cryptic. It would be nice if they would document how this actually worked, and what rules it followed. Otherwise, I feel like I'm just stabbing into the dark. That's not Yarn's problem, though it creates problems for Yarn for this very reason.
I'm happy to report the releasing a stable version using Lerna (and a small hack to libnpmpublish for good measure) resulted in the readme field being populated in the root metadata. Thanks again @Haroenv for putting me on the right path.
curl -s https://registry.npmjs.com/@antora/asciidoc-loader | python -m json.tool
Here's the hack I mentioned to set the readmeFilename. https://gitlab.com/antora/antora/-/blob/master/scripts/patch-libnpmpublish.js I reported it here: https://github.com/npm/libnpmpublish/issues/12
(I'm not sure if that patch helped, but it sure didn't hurt).
...and the READMEs are back!
https://yarnpkg.com/package/@antora/asciidoc-loader
Thank you again @Haroenv for your assistance. Not only did you help me resolve the problem at hand, you also made me aware of an underlying issue in my release process. So I'm in a better position now.
For packages that have a similar problem, but aren't able to release, I like the fix you suggested:
but extracted into a function & fall back to
pkg.versions[pkg['list-tags'].latest].readme
& same for readmeFilename
That seems to be the next step forward.
Yes, I'll get to that in the coming time, maybe the weekend
For some reason, the README file stopped showing for the following packages:
Is there a new packaging requirement for READMEs to display properly on yarnpkg.com?
At the time of this report, the most recent version is a prerelease with the npm label testing. Could that have something to do with it?