package.json contains a lot of things not necessary to end users

[steren]

Here's what we ask customers to copy paste in the quickstart:

{
  "name": "helloworld",
  "description": "Simple hello world sample in Node",
  "version": "1.0.0",
  "private": true,
  "main": "index.js",
  "scripts": {
    "start": "node index.js",
    "test": "mocha test/index.test.js --exit",
    "system-test": "NAME=Cloud test/runner.sh mocha test/system.test.js --timeout=30000",
    "lint": "eslint '**/*.js'",
    "fix": "eslint --fix '**/*.js'"
  },
  "author": "Google LLC",
  "license": "Apache-2.0",
  "dependencies": {
    "express": "^4.17.1"
  },
  "devDependencies": {
    "got": "^11.0.0",
    "mocha": "^8.0.0",
    "supertest": "^4.0.2"
  }
}

In practice, this could be boiled down to:

{
  "name": "helloworld",
  "description": "Simple hello world sample in Node",
  "version": "1.0.0",
  "private": true,
  "main": "index.js",
  "scripts": {
    "start": "node index.js"
  },
  "dependencies": {
    "express": "^4.17.1"
  }

The goal is the quickstart is to give a brief hello world experience, so devDependencies and additional scripts are not needed.

Similarly, author should not be Google, because we expect customers to copy paste it for their use.

[fhinkel]

@steren for which sample is this? AppEngine?

[steren]

Ah sorry, it's the sample used at https://cloud.google.com/run/docs/quickstarts/build-and-deploy#node.js

Which is https://github.com/GoogleCloudPlatform/nodejs-docs-samples/blob/master/run/helloworld/package.json

[averikitsch]

Hi Steren, we need the package.json to have this info in order to run tests (and meet legal requirements). Would you like to just override this in the docs?

[steren]

Could the internal test info be stored somewhere else?
For example in a package.internal.json file?

[steren]

@JustinBeckwith I wonder if this is something that you solved in other repos.

[JustinBeckwith]

Yeah, so taking a few steps back :) By a very large margin, developers are far more likely to consume samples via the embedded snippets available on pages of cloud.google.com. A very small percentage of users are actually cloning the repository, and looking at every single file. As such - we really do focus on the overall experience for how the snippets in this repository are embedded on cloud.google.com than we do on the merit of the standalone repository.

Now, that having been said - having snippets on cloud.google.com where we reference the package.json is exceedingly rare. This is the first time I've ever seen it, and to be honest ... I wonder out loud if it's even a good idea. Instead of trimming the package.json, I'd rather see us ask the user to bring their own project along for the ride, running the same commands I would to arrive here:

$ mkdir hello-run
$ cd hello-run
$ npm init -y
$ npm install express

From there, I would instruct the developer to add the start npm script:

"start": "node index.js"

And that's all we need! I don't think we need to provide a copy of the package.json we happen to use here. Thoughts?

[steren]

For now, we favored copy-pasting a 5-line package.json file over requiring to have npm installed, run 2 npm commands and edit the generated file.

But this is just one specific example, the broader question is that Google's internal testing sauce is not really useful when the repo is cloned, and should probably better be stored separately, or at least in different files or folders clearly marked as "internal".

[JustinBeckwith]

For the second bit - that was kind of the point I was trying to make. The number of users who clone this repository and try to run the code, or use it as an example is astonishingly low, as compared to folks who copy the snippets on cloud site. And in those cases - exposing how we test the snippets they use isn't strictly a bad thing :) We mostly care about what gets exposed on cloudsite. I'm entirely open to a discussion on how we optimize the cloudsite experience. I'm uninterested in changing how things are presented to users to clone the repo (because data).

[steren]

@averikitsch my worry of overriding it in the doc is that the package.json of the docs might become stale compared to the samples.

I'm not sure there is a solution to this discussion, so I'm fine closing as working as intended.

[averikitsch]

Right, I don't want the docs to get stale either. The Java quickstart sample does the Java equivalent of npm init and other samples have additional commands as well. I don't think it's bothersome to have the commands if it's normal dev experience for the language.

As for testing, we have found that users actually do run the tests and want to run the tests to make sure the sample is working as intended. We do make sure testing can be done both locally and on our system. We also make sure it follows best practices, so users can learn from our testing practices.

[steren]

For most regular samples, yes I agree that users want test. But here we are talking a Hello World, which is supposed to be the smallest, most simple, less scary code to print "hello".

We can switch to npm init, but that now requires npm installed on the local machine, which in the world of containers and minikube, should not have to be.

[grayside]

I think the existing package.json is too messy for cloudsite, and was a side effect of migrating to this repo where the testing infrastructure is partially centralized instead of entirely centralized.

Options here seem like:

1. Switch to npm init but as steren notes, that's creates a new dependency on local environment.
2. Add a special case to check for something like package.internal.json to .kokoro/build-with-run.sh
3. Have package.docs.json replicating package.json without the dev/script stuff.

[fhinkel]

I'm in favor of option 3. We just need to be aware that we won't get dependency updates. So if express goes to 5 at some point, we're out of luck. But I think that's OK.

[JustinBeckwith]

I still don't think this is an actual problem. Creating a secondary copy of the package json will absolutely lead to us missing things like dependency updates, changes to the npm scripts, etc. Wont this be a net negative?

[fhinkel]

For this particular repo, we render package.json in devsite docs. It does indeed look quite confusing with so much extra information. The simpler version has express as it's only dependecy. That's why I think we won't run into many missed dependency updates. It only has a start script, so again, not much room for updates.

[fhinkel]

But maybe correct is better than short, and we should keep it as is?

[JustinBeckwith]

If all that's really there is express, I guess it's ok 🤷 That thing has a semver major update once every other year :)

[steren]

Could an external system ensure that the package.json we use in docs is always a struct subset of the one we use in CI? This would avoid divergence in dependencies for example

[fhinkel]

Could an external system ensure that the package.json we use in docs is always a struct subset of the one we use in CI? This would avoid divergence in dependencies for example

That seems like a lot of overhead.

[averikitsch]

Now updated in the docs.