Skip to content

docgen: support hardhat3-style branches in injectTemplates#174

Open
ernestognw wants to merge 1 commit into
mainfrom
fix/docgen-hardhat3-injection
Open

docgen: support hardhat3-style branches in injectTemplates#174
ernestognw wants to merge 1 commit into
mainfrom
fix/docgen-hardhat3-injection

Conversation

@ernestognw
Copy link
Copy Markdown
Member

Summary

The docgen-pipeline (#140) assumed a Hardhat 2 / solidity-docgen source repo: docs/config.js, docs/templates-md/, kebab-case helpers, all CJS. The hardhat3 branch of openzeppelin-contracts ships an in-tree docgen plugin that:

  • reads docs/config.mjs (ESM, loaded via import() in prepare-docs.sh)
  • looks for templates in docs/templates/
  • defaults to pageExtension: '.adoc'
  • loads helpers only from helpers.ts / properties.ts via await import() — and only picks up ESM named function exports

Result on run 26575968521: our injection wrote docs/config.js and docs/templates-md/, both silently ignored. The plugin ran with the source repo's own AsciiDoc templates, copied .adoc files into content/contracts/latest/api/, fumadocs couldn't route them, and every guide xref into the API 404'd — 200 errors in next-validate-link.

What this PR does

  • Sniff the layout in injectTemplates. If the cloned repo has docs/config.mjs, treat it as the new layout; otherwise fall through to the legacy code path unchanged.
  • For the new layout:
    • Overwrite docs/config.mjs with docgen/config-md.mjs (forces pageExtension: '.mdx' and templates: 'docs/templates').
    • Wipe and re-populate docs/templates/, renaming helpers.js / properties.js to .cjs (the hardhat3 branch sets \"type\": \"module\" so plain .js would be treated as ESM and our module.exports would break). Patch the internal require('./helpers') in properties.cjs accordingly.
    • Emit thin ESM shim files helpers.ts / properties.ts that re-export each function — the new plugin's loader only walks .ts named exports.
  • Rename kebab-case helpers (oz-version, has-functions, typed-params, …) to camelCase across templates and helpers/properties modules. ESM named exports can't carry hyphens, and Handlebars is name-agnostic, so this is transparent to solidity-docgen on older tags.

Verified locally

Ran the updated generate-api-docs.js against the hardhat3 branch end to end (hardhat docgen + convert-adoc.js + pnpm run lint:links). 200 → 1 broken link. The residual is a pre-existing EIP712 NatSpec xref to /<base>/learn/upgrading-smart-contracts that also fails today on content/contracts/5.x/ — unrelated to this change.

Test plan

  • Re-run Generate API Docs - Contracts with tag_name: hardhat3 and trigger_type: commit; confirm the validate step is clean (or only flags the EIP712 learn-module xref).
  • Re-run Generate API Docs - Contracts with default tag_name: v5.6.1; confirm legacy path still works.
  • Spot-check the resulting PR against the live site: anchors like /contracts/latest/api/access#AccessManager-RoleRevoked-uint64-address- resolve.

🤖 Generated with Claude Code

The injectTemplates pipeline assumed a Hardhat 2 / solidity-docgen
layout (docs/config.js + docs/templates-md/). The hardhat3 branch of
openzeppelin-contracts replaces solidity-docgen with an in-tree plugin
that reads docs/config.mjs and templates from docs/templates/, emits
.adoc by default, and loads helpers/properties only from helpers.ts/
properties.ts via ESM dynamic import — so the previous injection was
silently ignored and AsciiDoc landed in content/, breaking every guide
xref into the API.

Detect the layout from the cloned repo and route accordingly:

- New layout: overwrite docs/config.mjs with our canonical config
  (forces pageExtension '.mdx' and templates path 'docs/templates'),
  wipe and re-populate docs/templates/, rename helpers.js / properties.js
  to .cjs (Node treats .js as ESM under "type": "module" on hardhat3),
  patch the internal require, and emit thin ESM shim helpers.ts /
  properties.ts so the in-tree plugin's loader picks up named exports.
- Legacy layout: unchanged behaviour.

Rename the kebab-case Handlebars helpers (oz-version, has-functions,
typed-params, …) to camelCase across the templates and helpers/
properties modules. ESM named exports cannot carry hyphens, and
Handlebars is name-agnostic, so this is transparent to solidity-docgen
on older tags.

Verified by running against the hardhat3 branch locally: 200 → 1 broken
links in next-validate-link, and the one residual is a pre-existing
EIP712 NatSpec xref to /<base>/learn/upgrading-smart-contracts that
also fails on current main's content/contracts/5.x.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ernestognw ernestognw requested a review from stevep0z as a code owner May 29, 2026 04:57
@netlify
Copy link
Copy Markdown

netlify Bot commented May 29, 2026

Deploy Preview for openzeppelin-docs-v2 ready!

Name Link
🔨 Latest commit ae4ad28
🔍 Latest deploy log https://app.netlify.com/projects/openzeppelin-docs-v2/deploys/6a191c9ea5b7d1000841c142
😎 Deploy Preview https://deploy-preview-174--openzeppelin-docs-v2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@ernestognw ernestognw requested a review from Amxx May 29, 2026 04:57
Comment on lines +147 to +152
return [
`import ${varName} from '${sourcePath}';`,
"",
...exports.map(e => `export const ${e} = ${varName}.${e};`),
"",
].join("\n");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is usually done using

Suggested change
return [
`import ${varName} from '${sourcePath}';`,
"",
...exports.map(e => `export const ${e} = ${varName}.${e};`),
"",
].join("\n");
return `export { `e.join(",")` } from "${sourcePath}";`;

Basically, that gives things like what we can find in ethers.ts

export {
    getAddress, getIcapAddress,
    getCreateAddress, getCreate2Address,
    isAddressable, isAddress, resolveAddress
} from "./address/index.js";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is assuming we don't need the varName to be declared and its only used as an intermediary

Comment on lines +164 to +168
let newLayout = false;
try {
await fs.access(path.join(tempDir, "docs", "config.mjs"));
newLayout = true;
} catch {}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let newLayout = false;
try {
await fs.access(path.join(tempDir, "docs", "config.mjs"));
newLayout = true;
} catch {}
const newLayout = await fs.access(path.join(tempDir, "docs", "config.mjs")).then(() => true, () => false);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants