chore(repo): Add logic to prevent duplication of legacy hooks Typedoc#8074
chore(repo): Add logic to prevent duplication of legacy hooks Typedoc#8074SarahSoutoul wants to merge 6 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: a038f3e The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
SarahSoutoul
changed the title
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
📝 WalkthroughWalkthroughAdds legacy-hook handling to the TypeDoc extraction script (.typedoc/extract-returns-and-params.mjs): introduces 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.typedoc/extract-returns-and-params.mjs:
- Around line 40-45: Extract the duplicated legacy hook detection and
filename/path rewriting into a single helper (e.g., isLegacyHook(fileName) and
rewriteLegacyOutput(fileName, dirName) or a single getLegacyOutput(fileName,
dirName)) and use it from extractReturnsSection, extractParametersSection and
moveLegacyHookDoc; the helper should encapsulate the logic that currently checks
fileName === 'use-sign-in-1' || fileName === 'use-sign-up-1', produces the
outputBaseName by stripping /-1$/ and appends '-return', and returns the new
outputDir (path.join(dirName, 'legacy')) and ensures fs.mkdirSync(..., {
recursive: true }) is called once; update those three call sites to use the
helper instead of duplicating the checks and rewrites.
- Around line 126-129: Update the JSDoc for the function that handles legacy
hook doc moves (the block currently stating "Renames legacy hook docs to use a
-legacy suffix" with `@param` {string} filePath) to accurately describe what the
code does: it moves files into a legacy/ directory and strips a trailing "-1"
from filenames rather than appending a "-legacy" suffix; ensure the description
and any examples reflect "move into legacy/ and remove -1" so future maintainers
won't be misled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 76db7019-d139-4c33-8890-18be580056e3
📒 Files selected for processing (1)
.typedoc/extract-returns-and-params.mjs
|
awesome fix! but for the current useSignIn and useSignUp hooks, they're not returning the output we'd actually want in the docs: we'd want the signupsignalvalue type to be a table of those returns: |
@alexisintech Yeah that was always the case as they're not JSDoc documented properly, and currently the docs don't use these (they're manually typed). I think I'd prefer to keep the scope of this PR to just fixing the duplication, and can create another PR to make sure the output is good for those if that's okay. |
SarahSoutoul
changed the title
SarahSoutoul
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.typedoc/extract-returns-and-params.mjs:
- Around line 57-65: When legacyTarget is set the script moves outputs to
legacy/ but does not remove any existing stale files with the old "-1" suffix,
leaving duplicates (e.g. use-sign-in-1-return.mdx); update the logic around
outputBaseName/outputDir/newFilePath to detect and delete those stale artifacts
before writing: when legacyTarget is true compute the originalLegacyPath(s) (the
prior filePath + `${legacyTarget.baseName}-1-return.mdx` and `-1-params.mdx` or
any `*-1-*.mdx` for that base name) and remove them (fs.unlinkSync or fs.rmSync
with exists checks) and/or remove any `*-1-*.mdx` in the original outputDir,
then continue to create the newFilePath in legacyTarget.outputDir as now. Ensure
this cleanup runs only when legacyTarget is truthy to avoid accidental
deletions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: a9713ab3-ceca-4322-8997-139cddc57fdc
📒 Files selected for processing (1)
.typedoc/extract-returns-and-params.mjs
ohhh the current docs don't use them - okay sounds good! |
wobsoriano
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/short-apes-joke.md:
- Around line 1-2: The changeset file ".changeset/short-apes-joke.md" is empty
(only '---' delimiters); update it to declare the release intent by replacing
the empty frontmatter with a YAML changeset that lists the packages to bump
(each as package-name@version or package-name: patch/minor/major) and a short
summary message describing the change; ensure the file has the required opening
and closing '---' surrounding the package bump entries and the summary so
release/changelog tooling can parse it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: ca16d884-5dae-485a-8ece-ad878dbac729
📒 Files selected for processing (1)
.changeset/short-apes-joke.md
3 participants
Description
The whole reason for this PR is explained in this Slack comment/ thread. We introduced the extract script to split out Returns/Parameters into standalone Typedoc files. That was working when the legacy
useSignIn/useSignUphooks were the only ones exposed. Now that the newuseSignIn/useSignUphooks are no longer@internal,Typedoc generates outputs for both the new and legacy hooks. This causes:use-sign-in.mdx,use-sign-up.mdx,use-sign-in-return.mdxetc.)use-sign-in-1*.mdx/use-sign-up-1*.mdxTo prevent this duplication, here are the changes I applied.
Changes
The extractor now detects the legacy hook outputs and:
.typedoc/docs/react/legacy/-1suffixLegacy docs are now written to:
.typedoc/docs/react/legacy/use-sign-in.mdx.typedoc/docs/react/legacy/use-sign-up.mdxLegacy returns/params are now written to:
The new signal‑based hooks keep the normal canonical paths:
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit