chore: refactor starters fetching to use dynamic source from starter repo#492
chore: refactor starters fetching to use dynamic source from starter repo#492maximepvrt wants to merge 4 commits intonuxt:mainfrom
starter repo#492Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
@maximepvrt is attempting to deploy a commit to the Nuxt Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe PR converts starters data from a static, synchronous export to an async server route that fetches and builds Starter objects from GitHub contents at runtime. The Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 3
🤖 Fix all issues with AI agents
In `@server/routes/data/starters.json.ts`:
- Line 1: The handler exported from defineEventHandler declares an unused
parameter `event` and uses untyped fetch results causing implicit any for
`files` and `file`; rename the parameter to `_event` (or remove it), call $fetch
with a type parameter to declare the expected response shape (e.g.,
$fetch<YourResponseType>(...)) so `files` is strongly typed, and update the .map
callback to use a typed `file` parameter (e.g., (file: FileType) => ...)
matching that response type; adjust any related type aliases/interfaces used by
the handler to match the fetched data structure.
- Line 15: The current templates.push call sets slug: templateName before
spreading ...template which allows a slug from the fetched template object to
overwrite the filename-derived templateName; update the object literal in the
templates.push expression so that the spread comes first and then slug:
templateName appears after (e.g., change to { ...template, slug: templateName,
tar: ... }) to ensure the filename-derived slug wins; locate the
templates.push(...) expression and reorder the properties accordingly
(referencing templateName, template, and the Starter-shaped fetched JSON).
- Around line 6-17: Reformat the callback passed to Promise.all(files.map(...))
to use the project's 2-space indentation (instead of the current 6/8-space),
adjusting the entire block beginning with "async (file) => {" through the
closing "}))" so each nested line (checks for file.download_url/type/name, const
templateName, const template fetch, and the templates.push) is indented by 2
spaces per level; verify the async callback body, the if blocks, and the object
literal for templates.push maintain consistent 2-space indentation so the linter
stops failing.
🧹 Nitpick comments (2)
shared/types/index.ts (1)
8-9: Nit: Inconsistent separators in the interface.Lines 8–9 use trailing commas while every other property uses no separator. Consider removing them for consistency.
✏️ Suggested fix
- repo: string, - branch: string, + repo: string + branch: stringserver/routes/data/starters.json.ts (1)
4-4: Unauthenticated GitHub API calls are subject to strict rate limits.The GitHub Contents API allows only 60 requests/hour for unauthenticated callers. This handler makes 1 + N requests (directory listing + one per template file) on every cache miss. With ISR at 3600s that's likely fine under normal conditions, but any cache purge, deployment, or cold-start burst could exhaust the quota quickly.
Consider either:
- Using a
GITHUB_TOKENvia a server-side env variable to raise the limit to 5,000 req/hour.- Fetching the entire directory tree in a single call (e.g., Git Trees API with
?recursive=1) to reduce request count.
server/routes/data/starters.json.ts
Outdated
| export default defineEventHandler(async () => { | ||
| const templates: Starter[] = [] | ||
|
|
||
| const files = await $fetch('https://api.github.com/repos/maximepvrt/nuxt-starter/contents/templates?ref=update-template') |
There was a problem hiding this comment.
use https://api.github.com/repos/nuxt/starter/contents/templates?ref=templates after merge nuxt/starter#1774
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@server/routes/data/starters.json.ts`:
- Around line 1-24: Wrap the external GitHub calls inside robust error handling:
replace the Promise.all over files with Promise.allSettled and for each entry in
the map around the inner $fetch (the call that produces template in the
defineEventHandler) add a try/catch so one failed template is skipped and logged
(push only successful, non-deprecated templates into the templates array); also
wrap the initial $fetch that retrieves files in its own try/catch and return an
empty templates array or a safe fallback on failure; finally, when calling
$fetch to GitHub include an Authorization header using a server-side
GITHUB_TOKEN env var to increase rate limits.
- Line 4: The $fetch call that assigns files is pointing at a hardcoded fork and
feature branch; update the URL passed to $fetch (the string literal) to the
canonical upstream repository and branch (e.g., replace
'https://api.github.com/repos/maximepvrt/nuxt-starter/contents/templates?ref=update-template'
with the correct upstream repo and branch such as
'https://api.github.com/repos/nuxt/starter/contents/templates?ref=main'), or
better yet make the repo and ref configurable via an environment variable and
use that in the $fetch call so the files variable always targets the canonical
repo/branch in production.
🧹 Nitpick comments (1)
server/routes/data/starters.json.ts (1)
11-13: Unsafeas Startercast on unvalidated external data.The fetched JSON is cast to
Starterwithout any runtime validation. If the remote JSON schema changes or a file is malformed, the code will silently produce incorrectStarterobjects (e.g., missingrepoorbranchwould yield a brokentarURL on Line 15).Consider adding minimal runtime validation (e.g., check that
template.repoandtemplate.branchare truthy strings) before pushing.
| export default defineEventHandler(async () => { | ||
| const templates: Starter[] = [] | ||
|
|
||
| const files = await $fetch('https://api.github.com/repos/maximepvrt/nuxt-starter/contents/templates?ref=update-template') | ||
|
|
||
| await Promise.all(files.map(async (file) => { | ||
| if (!file.download_url || file.type !== 'file' || !file.name.endsWith('.json')) { | ||
| return | ||
| } | ||
| const templateName = file.name.replace('.json', '') | ||
| const template = await $fetch(file.download_url, { | ||
| responseType: 'json', | ||
| }) as Starter | ||
| if (!template.deprecated) { | ||
| templates.push({ slug: templateName, ...template, tar: `https://codeload.github.com/${template.repo}/tar.gz/refs/heads/${template.branch}` }) | ||
| } | ||
| })) | ||
|
|
||
| return templates.sort((a, b) => { | ||
| if (a.default && !b.default) return -1 | ||
| if (!a.default && b.default) return 1 | ||
| return 0 | ||
| }) | ||
| }) |
There was a problem hiding this comment.
No error handling for external GitHub API calls.
Both $fetch calls (directory listing on Line 4, individual template files on Line 11) can fail due to rate-limiting (unauthenticated GitHub API allows 60 req/hr), network errors, or malformed responses. A single failure inside Promise.all will reject the entire response with an unhandled error.
Consider:
- Adding a
try/catcharound each inner fetch so one bad template doesn't take down the whole endpoint. - Using
Promise.allSettledinstead ofPromise.all. - Authenticating the GitHub API request (e.g., via a server token) to raise the rate limit, especially given ISR revalidation every hour.
🧰 Tools
🪛 GitHub Check: test
[failure] 15-15:
'slug' is specified more than once, so this usage will be overwritten.
[failure] 6-6:
Parameter 'file' implicitly has an 'any' type.
[failure] 6-6:
'files' is of type 'unknown'.
[failure] 15-15:
'slug' is specified more than once, so this usage will be overwritten.
[failure] 6-6:
Parameter 'file' implicitly has an 'any' type.
[failure] 6-6:
'files' is of type 'unknown'.
🤖 Prompt for AI Agents
In `@server/routes/data/starters.json.ts` around lines 1 - 24, Wrap the external
GitHub calls inside robust error handling: replace the Promise.all over files
with Promise.allSettled and for each entry in the map around the inner $fetch
(the call that produces template in the defineEventHandler) add a try/catch so
one failed template is skipped and logged (push only successful, non-deprecated
templates into the templates array); also wrap the initial $fetch that retrieves
files in its own try/catch and return an empty templates array or a safe
fallback on failure; finally, when calling $fetch to GitHub include an
Authorization header using a server-side GITHUB_TOKEN env var to increase rate
limits.
server/routes/data/starters.json.ts
Outdated
| export default defineEventHandler(async () => { | ||
| const templates: Starter[] = [] | ||
|
|
||
| const files = await $fetch('https://api.github.com/repos/maximepvrt/nuxt-starter/contents/templates?ref=update-template') |
There was a problem hiding this comment.
Hardcoded fork URL — must point to the canonical repo before merging.
This fetches from maximepvrt/nuxt-starter on a feature branch. Before merging, update to the target upstream repo and branch (e.g., nuxt/starter).
🤖 Prompt for AI Agents
In `@server/routes/data/starters.json.ts` at line 4, The $fetch call that assigns
files is pointing at a hardcoded fork and feature branch; update the URL passed
to $fetch (the string literal) to the canonical upstream repository and branch
(e.g., replace
'https://api.github.com/repos/maximepvrt/nuxt-starter/contents/templates?ref=update-template'
with the correct upstream repo and branch such as
'https://api.github.com/repos/nuxt/starter/contents/templates?ref=main'), or
better yet make the repo and ref configurable via an environment variable and
use that in the $fetch call so the files variable always targets the canonical
repo/branch in production.
maximepvrt
changed the title
starters repostarter repo
1 participant
❓ Type of change
📚 Description
I would like to propose replacing the static
starters.jsonfile in this repository with a dynamic version fetched directly from thenuxt/starterrepository, similar to how the Nuxt CLI operates.The current static approach leads to synchronization issues. I noticed this when @benjamincanac updated the UI templates:
npm create nuxt@latest -- -t uicorrectly uses the updated template.nuxt.newstill opens an outdated repository because the localstarters.jsonwas not updated.To facilitate this transition, I have already modified the repository definitions within
nuxt/starterand add properties to this repo with this PR : nuxt/starter#1774.Questions & Discussion Points
1. Template Recursion & Subdirectories
I noticed that subdirectories in the templates branch (e.g.,
uiandui-vue) are not currently integrated into the CLI because there is no recursion logic.Currently, @benjamincanac uses the following command for UI templates in the README:
npm create nuxt@latest -- -t github:nuxt-ui-templates/editor2. Centralizing the Source of Truth
To avoid maintaining starter lists in two places, should we move the
starters.jsondefinition to thenuxt.comrepository? This would allow both the CLI and the nuxt.new to pull from a single source.3. UX Consolidation
I am also questioning the placement of the starter cards. Would it make more sense to migrate/repatriate the
nuxt.newstarter cards to the top of the nuxt.com/templates page for a more unified user experience?