Repository navigation
Conversation
|
@yoreva-01 is attempting to deploy a commit to the nucliweb's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Thanks, this is a big one: it touches a part of the docs we still have to decide on (how the returned object is generated and kept up to date), so it will take a closer review. I'll come back to it. |
nucliweb
left a comment
There was a problem hiding this comment.
Thanks for taking this on, and for keeping it to one category as the plan in #75 suggests: a manual capture script that reuses runSnippets, results committed as JSON, and the same examples in the skill references. The capture script fails on page errors and on any status other than ok, which is what we want. A few changes before it can go in:
1. The source JSON lives in a generated directory. skills/, .claude/skills/ and dist/ are output of npm run generate-skills and are never edited by hand, but the examples are written to skills/webperf-core-web-vitals/references/examples.json, the pages import them from there, and the generator reads them back from its own output. Please keep the source next to the snippets, as snippets/CoreWebVitals/examples.json (the same pattern as WORKFLOWS.md), import it from the pages, and let generate-skills.js copy it into the references of the skill.
2. Each example should show the snippet doing its job. On core-web-vitals.html the LCP element is the <h1>, so LCP-Image-Entropy returns lcpImage: null, LCP-Video-Candidate returns isVideo: false, and LCP-Subparts shows a text LCP with no resource phases. A reader learns little from the "not applicable" case. The e2e tests already have pages for this, so there is no need for a new fixture:
- The
/seededpage ofstartContractServers()incli/tests/helpers/contract.js(the page #75 refers to) has an LCP image (#hero), a layout shift at 300 ms and a button with a slow handler: it fits LCP, LCP-Subparts, LCP-Trail, LCP-Image-Entropy, CLS and INP (with a click on#go). cli/tests/fixtures/video-poster.htmlhas a video as the LCP element, for LCP-Video-Candidate.
A small map from snippet to page and interaction steps in the capture script would cover both.
3. The "Returned object" heading belongs to the page. The <h3> is rendered inside the Snippet component, so it does not appear in the table of contents of the page and has no anchor. Please render only the example in the component (or in a small component of its own) and write the ### Returned object heading in each MDX page, after the snippet. The fields from SCHEMA.md can come in a later pull request; the example alone is a good first step.
4. No special case for the category in the generator. generate-skills.js checks category === 'CoreWebVitals'. If it reads snippets/<Category>/examples.json whenever the file exists, the other categories work without touching the generator again.
5. A test that keeps the examples honest. If a snippet changes, a committed example can drift from what the snippet returns without anything failing. A unit test over examples.json would catch the obvious cases: every snippet of the category has an example, script matches the name of the snippet, and status is ok. The values themselves vary between runs, which is fine.
One more thing: @ManuelPauloAfonso offered in #75 to implement this and asked which category to start with. Manuel, if you are still interested, the next categories (Loading, Interaction, Media) are open once this one sets the pattern.
Thanks again, the structure is close to what we need.
c749658 to
bd69458
Compare
|
Thanks for the detailed review @nucliweb! I have rebased onto the latest \main\ and addressed all five feedback points:
|
Summary
Validation
npm run capture:examplescaptured all seven results in Chromium.npm run lintpassed.npm exec -- vitest run components/Snippet.test.jsxpassed (9 tests).npm exec -- vitest run cli/tests/e2e/cwv.test.jspassed (3 tests).npm run test:sitepassed (44 tests).npx next build --webpackcompiled and generated all 69 static pages.git diff --checkpassed.