docs(node): make quickstart runnable end to end - #394
Closed
JoshKappler wants to merge 1 commit into
Closed
Conversation
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type: fix. The Node quickstart cannot be completed as written. A reader who follows it top to bottom hits a hard error on the first code sample and never reaches a running service.
This quickstart was created in the #354 docs refactor merged July 16 and had not been walked end to end yet. The docs bots do mechanical sync and grammar passes (open PR #390 is a grammar-only pass on a sibling quickstart page), so first-run defects like this survive them.
What is broken
Following the page's own steps produces a project whose
package.jsonhas"type": "commonjs", which is whatnpm init -ywrites on current npm. The first sample uses ESMimportsyntax, sonode server.jsfails immediately:Three more gaps stop the page short of a running service:
import express, but the install step only installsgt-nodeandgt.app.listen, and no step runs or checks the server, so even with the imports fixednode server.jsexits silently.npx gt translatedead-ends: it requires agt.config.jsonand credentials that the page never creates, and it points atgt init, which the page never mentions.What this changes
"type": "module".expressto the install commands in all four package managers.app.listento the final server block and a new "Run and verify" step (node server.jsplus acurland the expected response).npx gt initfirst to creategt.config.jsonand credentials.tswhile titledserver.js; the code is plain JavaScript.Verified by scaffolding the page from scratch in a clean directory:
npm init -y, set"type": "module", install the dependencies, paste the assembled server code,node server.js, thencurl. The server returnsHTTP 200with{"message":"Hello, Alice!"}(the source string, since no credentials are set). The repo CI checks pass locally: unsafe-HTML validation, link validation, andpnpm build:content.On the obvious counterargument
Node's error message does name the fix for the ESM problem, so an experienced reader could add
"type": "module"on their own. That holds for that one line and not for the rest: the missingexpressinstall, the missingapp.listen, and thegt translateprerequisites give no such hint, and a quickstart should not make the reader debug its first sample. The page still states the ESM line so the happy path runs with no detours.Coordination with #389
Draft PR #389 is editing this same file's step 2 (credentials and environment variables), blocked on a
gt-noderelease. I kept my scope to page structure and left step 2 untouched. One consequence: the step 2 code block keeps itstsfence, so after this lands the page has onets-fenced block and twojs-fenced blocks. Aligning that last fence belongs with #389's rewrite of that block, so I note it here rather than editing a region #389 is actively changing.There is also a merge-order interaction. This PR's new "Run and verify" step expects the handler to echo the source string, which is correct on current main where step 2 sets no API key. #389 rewrites step 2 to export a real dev key, and a dev key can trigger on-demand runtime translation instead of a source-string echo, which would make the verify step's expected output wrong. Two clean orders: land #389 first, then a one-line follow-up here adjusting the verify wording; or land this PR first (correct on current main) and make that same one-line adjustment when #389 lands. Either way it is a single follow-up line, not a rebase conflict.
If I have the framing wrong
If the quickstart intentionally assumes the reader already has an Express app and a configured project, I'm happy to rescope to that framing instead, for example dropping the install and run steps and stating the assumption up front.