dx: improve local docs quickstart with bootstrap + smart pipeline runner#67
dx: improve local docs quickstart with bootstrap + smart pipeline runner#67isyedrayan1 wants to merge 1 commit intowebpack:mainfrom
Conversation
|
|
There was a problem hiding this comment.
Pull request overview
Improves the local contributor workflow for generating the webpack API docs by adding a bootstrap step for checking out the upstream webpack source at the pinned HEAD_COMMIT, plus a single-command pipeline runner with skip logic.
Changes:
- Added
scripts/bootstrap-webpack.mjsto clone/update./webpackat the commit pinned inHEAD_COMMIT. - Added
scripts/docs-pipeline.mjsto run bootstrap → markdown generation → HTML build with--force/--verbose/--no-htmlsupport and stateful skipping. - Updated
generate-md.mjs,package.jsonscripts, andREADME.mdto support and document the new local quickstart flow.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/docs-pipeline.mjs | New ordered pipeline runner with preflight checks and skip/state logic. |
| scripts/bootstrap-webpack.mjs | New helper to clone/fetch/checkout upstream webpack at HEAD_COMMIT. |
| generate-md.mjs | Adds local preflight checks for presence of required webpack/ inputs. |
| package.json | Adds bootstrap:webpack, docs:quickstart, and docs:doctor scripts. |
| package-lock.json | Lockfile updates from dependency resolution/install. |
| README.md | Adds local quickstart, troubleshooting, and contributor workflow docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Useful flags for the quickstart runner: | ||
|
|
||
| - `--force` reruns every step | ||
| - `--verbose` prints detailed diagnostics | ||
| - `--no-html` skips the HTML build | ||
|
|
||
| Example: | ||
|
|
||
| ```bash | ||
| node scripts/docs-pipeline.mjs --force --verbose | ||
| ``` |
There was a problem hiding this comment.
The README describes flags for the quickstart runner but the example uses node scripts/docs-pipeline.mjs .... If the intended entrypoint is npm run docs:quickstart, passing flags requires npm run docs:quickstart -- --force --verbose. Consider updating the example (or adding a second example) so contributors don’t try npm run docs:quickstart --force and get unexpected npm behavior.
| if (force || currentWebpackCommit !== targetCommit) { | ||
| log('Step 1/3: bootstrap webpack source'); | ||
| run('node', ['scripts/bootstrap-webpack.mjs']); | ||
| } else { | ||
| log('Step 1/3: bootstrap webpack source (skipped; already at target commit)'); | ||
| } | ||
|
|
||
| const webpackMajor = readWebpackMajorVersion(); | ||
| const docsDir = resolve(ROOT, `pages/v${webpackMajor}.x`); |
There was a problem hiding this comment.
Step 1 can be reported as "skipped" even when the local webpack checkout is incomplete (e.g., webpack/package.json or webpack/types.d.ts missing). In that case the pipeline will fail later with a confusing error even though bootstrap was skipped. Consider extending the skip condition to also verify the required webpack files exist (and/or reuse the same checks as generate-md.mjs) before deciding to skip bootstrap.
| resolve(ROOT, 'generate-md.mjs'), | ||
| resolve(ROOT, 'tsconfig.json'), | ||
| resolve(ROOT, 'plugins'), | ||
| resolve(ROOT, 'HEAD_COMMIT'), |
There was a problem hiding this comment.
sourceFingerprint() currently tracks repo files (generate-md.mjs/tsconfig/plugins/HEAD_COMMIT) but not the local webpack inputs used for generation (webpack/types.d.ts, webpack/package.json). This means Step 2 can be skipped even if a contributor edits webpack types locally (without changing commits), leaving stale markdown. Include the relevant webpack/ files in the fingerprint (or in needsGenerate) so local edits force regeneration.
| resolve(ROOT, 'HEAD_COMMIT'), | |
| resolve(ROOT, 'HEAD_COMMIT'), | |
| resolve(WEBPACK_DIR, 'types.d.ts'), | |
| WEBPACK_PKG, |
| const result = spawnSync(command, commandArgs, { | ||
| cwd: ROOT, | ||
| encoding: 'utf8', | ||
| }); | ||
|
|
||
| if (result.status !== 0) { | ||
| const fullCommand = [command, ...commandArgs].join(' '); | ||
| fail(`Command failed: ${fullCommand}`); | ||
| } | ||
|
|
||
| return result.stdout.trim(); |
There was a problem hiding this comment.
runCapture() discards stderr and only reports "Command failed: ..." on non-zero exit. For failures like a corrupt git repo or missing git, this can hide the underlying error message and make troubleshooting harder. Consider including result.stderr (and result.error for ENOENT) in the failure output so errors are actionable.
| const result = spawnSync(command, commandArgs, { | |
| cwd: ROOT, | |
| encoding: 'utf8', | |
| }); | |
| if (result.status !== 0) { | |
| const fullCommand = [command, ...commandArgs].join(' '); | |
| fail(`Command failed: ${fullCommand}`); | |
| } | |
| return result.stdout.trim(); | |
| const fullCommand = [command, ...commandArgs].join(' '); | |
| const result = spawnSync(command, commandArgs, { | |
| cwd: ROOT, | |
| encoding: 'utf8', | |
| }); | |
| // Handle spawn errors (for example, command not found). | |
| if (result.error) { | |
| if (result.error.code === 'ENOENT') { | |
| fail(`Command not found: ${command} (while running: ${fullCommand})`); | |
| } | |
| fail(`Failed to run command: ${fullCommand}\n${result.error.message}`); | |
| } | |
| if (result.status !== 0) { | |
| let errorDetail = ''; | |
| if (result.stderr) { | |
| const stderrString = | |
| typeof result.stderr === 'string' | |
| ? result.stderr | |
| : result.stderr.toString('utf8'); | |
| const trimmedStderr = stderrString.trim(); | |
| if (trimmedStderr) { | |
| errorDetail = '\n' + trimmedStderr; | |
| } | |
| } | |
| fail( | |
| `Command failed with exit code ${result.status}: ${fullCommand}${errorDetail}`, | |
| ); | |
| } | |
| const stdoutString = | |
| typeof result.stdout === 'string' | |
| ? result.stdout | |
| : (result.stdout || '').toString('utf8'); | |
| return stdoutString.trim(); |
|
|
||
| if (result.status !== 0) { | ||
| const fullCommand = [command, ...args].join(' '); | ||
| throw new Error(`Command failed: ${fullCommand}`); |
There was a problem hiding this comment.
In runCapture(), failures throw Error("Command failed: ...") without surfacing the command’s stderr (and without checking result.error when the executable is missing). This can make bootstrap failures hard to diagnose. Consider appending result.stderr / result.error?.message to the thrown error to keep the message actionable.
| throw new Error(`Command failed: ${fullCommand}`); | |
| const details = []; | |
| if (result.error && result.error.message) { | |
| details.push(`Error: ${result.error.message}`); | |
| } | |
| if (typeof result.stderr === 'string' && result.stderr.trim() !== '') { | |
| details.push(`stderr:\n${result.stderr.trim()}`); | |
| } | |
| const detailSuffix = details.length > 0 ? `\n${details.join('\n')}` : ''; | |
| throw new Error(`Command failed: ${fullCommand}${detailSuffix}`); |
| if (!existsSync('./webpack')) { | ||
| throw new Error('Webpack source not found. Run: npm run bootstrap:webpack'); | ||
| } | ||
|
|
||
| if ( | ||
| !existsSync('./webpack/package.json') || | ||
| !existsSync('./webpack/types.d.ts') | ||
| ) { | ||
| throw new Error( | ||
| 'Webpack source is incomplete. Run: npm run bootstrap:webpack' | ||
| ); | ||
| } |
There was a problem hiding this comment.
generate-md.mjs now checks that webpack/ exists, but it can still generate docs from a webpack checkout that’s not the HEAD_COMMIT pinned commit (e.g., if a contributor has webpack/ at a different SHA). That can produce pages that don’t match CI’s docs-check behavior and will fail in PRs. Consider validating git -C ./webpack rev-parse HEAD against HEAD_COMMIT and erroring with an actionable message when they differ.
avivkeller
left a comment
avivkeller
left a comment
There was a problem hiding this comment.
I'm sorry, but I really have no idea what it is this PR is trying to accomplish.
|
Most of what this PR adds can simply be: |
|
@avivkeller This PR is simply making the project setup easier for every contributors. With custom scripts. To run the project repo locally.without getting confused with the commands and pre-requisites as I've faced few issues while running the project. |
|
Thanks for the feedback, that makes sense. I see that the PR added unnecessary complexity and wasn’t aligned with the core goals of the project. I’ll refocus on smaller, more relevant contributions directly related to the pipeline and theme output. Appreciate the clarification. |
Summary
This PR improves local contributor experience for the docs pipeline without changing core generation behavior.
What changed
Added npm scripts:
Scope and non-goals
Validation
[docs:quickstart] Step 1/3: bootstrap webpack source
[bootstrap:webpack] Cloning webpack repository...
[bootstrap:webpack] Checked out b5499e05b0c1c2847545a610cbaea3d352328d3d.
[docs:quickstart] Step 2/3: generate markdown docs
�[96m[info]�[0m Loaded plugin typedoc-plugin-markdown
�[96m[info]�[0m Loaded plugin /workspaces/webpack-doc-kit/plugins/processor.mjs
�[96m[info]�[0m Loaded plugin /workspaces/webpack-doc-kit/plugins/theme/index.mjs
�[96m[info]�[0m markdown generated at ./pages/v5.x
[docs:quickstart] Step 3/3: build html docs
[docs:quickstart] Step 1/3: bootstrap webpack source (skipped; already at target commit)
[docs:quickstart] Step 2/3: generate markdown docs (skipped; no relevant changes)
[docs:quickstart] Step 3/3: build html docs (skipped; output is up to date)
Why
CI already checks out webpack automatically, but local contributors were missing this setup path. This PR makes local setup explicit, reproducible, and fast for repeat runs.