Use Hugo as a peerDependency instead of hiding it

[SayakMukhopadhyay]

This issue is related to the PR #2140, #2163 and #2164

Those PRs were created to hide the hugo dependency as it was creating issues in kubernetes/website#48724. I found that the issue was not strictly due to Hugo being present, rather it was because a dependency required for Hugo was not present in the container (see the convo in kubernetes/website#48724 (comment)).

But at the same time, I can understand that installing Hugo is not required when installing Docsy via NPM as most users will install their own version of Hugo. Moreover based on this comment

          ...We'll consider implementing this, or not, at `HEAD` if when the need arises.

Originally posted by @chalin in #2140 (comment)

I suggest we move Hugo to peerDependency instead of using disabledDependency (which is not a valid property, but I guess it's a stopgap solution). Moreover, I don't think explicitly doing this to every earlier version of docsy is required as the problem in the kubernetes/website wasn't due to the dependency in the first place.

[chalin]

Hi @SayakMukhopadhyay - thanks for the issue, the background info, and the Docsy upgrade work you've been doing 🙌🏻.

Regarding the issue you've raised here, this is my understanding:

• When Docsy included as an NPM dependency via GitHub like this "docsy": "github:google/docsy#semver:0.6.0", as you have done in Upgrade Docsy to 0.6 kubernetes/website#49416, then NPM will always build the NPM package.

• NPM builds packages in a "dev" environment and thus pulls in dev dependencies, and as of NPM 7+, peer dependencies as well:

  Automatically installing peer dependencies is an exciting new feature introduced in npm 7.

So unless you've tested this in some other context and can confirm that it works for you, it is my understanding that declaring hugo-extended as a peer dependency won't prevent it from being loaded by NPM. I'd be glad to be proven wrong.

[SayakMukhopadhyay]

Yeah, I was wrong about newer npm not installing peer deps by default. It seems like it does, especially when doing npm i. But having Hugo as a peer dep or a dev dep would mean that consumers of docsy can use npm i --omit peer docsy or npm i --omit dev docsy to ensure that Hugo is not installed. Moreover, running npm ci with NODE_ENV set to production will not install dev dependencies by default. This direction can be added into the docs and imo is a better solution as its a documented feature of npm.

Disregard my earlier comments(except maybe the fact about me being wrong about npm installing peer deps by default). It seems like no matter if hugo is added as a peer dependency or if --omit is used or if NODE_ENV is set, all of them come with issues. Using --omit means that not only do I have to use npm I --omit dev but I also have to use npm ci --omit dev which is troublesome in the case where I only want to omit the dev dependencies of only some dependencies. Similarly setting NODE_ENV=production won't work as docsy itself is a dev dependency and it won't get installed unless it is added as a direct dependency.

The only thing that works reliably is npm ci --ignore-scripts. And all of this is due to hugo-extended having a postinstall script which installs hugo (see https://github.com/jakejarvis/hugo-extended/blob/7cd22ad87d0c53500016ca4de7bb73908c03d98e/package.json#L41). At this point, I am not sure what would be the best way forward without implementing workarounds only for the k8s websites. One way could be to add a newer version of hugo into the package.json of the website itself and not download and build hugo manually. Although that too might come with its issues as the hugo-extended npm lib is community maintained and not from Hugo. I will discuss this with the k8s maintainers and update.

[chalin]

I'm glad that you are arriving at the same conclusions as I did. Even using --ignore-scripts causes problems.

I'm going close this issue given that a shift to peer deps isn't a solution, and because you've found that including gcompat in the container image allows Docsy to be built as a (dev) NPM package (thanks again for find that solution!), and so Docsy can be included as a dependency via github:google/docsy#semver:0.6.0. I'll create a separate issue containing my ideas moving forward.

[SayakMukhopadhyay]

Yeah, those are exactly my thoughts regarding consuming docsy. If you do create the issue with your ideas, can you please link this issue or ping me. That would be much appreciated and I will try working on the fix if you would like so.

[chalin]

Here it is: #2172