-
Notifications
You must be signed in to change notification settings - Fork 8
Add elements manifest and JSDoc generation #692
Conversation
🦋 Changeset detectedLatest commit: c1d86c2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
87da386 to
3a75d56
Compare
0c239c3 to
1235045
Compare
| * But just because something is in a component's JSDoc doesn't mean it | ||
| * is correct or exists. A developer, for example, may add a `@slot` comment, | ||
| * then later forget to remove it after removing the slot. | ||
| * |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Button's label slot comment is an example. Button hasn't had label slot for a while.
| "stylelint-use-nesting": "^6.0.0", | ||
| "terser": "^5.37.0", | ||
| "ts-lit-plugin": "^2.0.2", | ||
| "ts-morph": "^25.0.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ts-morph can't claim to be well documented. But it's widely used and works well. I initially used jscodeshift. But it took two or three times the code and didn't play as nicely with TypeScript.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow that's a lot of usage!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About half of jscodeshift. But still enough to give us at least some confidence that it'll be around for a while.
| exclude: [ | ||
| './src/**/*stories*', | ||
| './src/**/*test*', | ||
| './src/cem-analzyer-plugins/**', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many of these globs go away with a monorepo. So does importing from ./dist above.
| for (const attribute of declaration.attributes) { | ||
| if (attribute.name === 'version') { | ||
| attribute.type = { | ||
| text: packageJson.version, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| return; | ||
| } | ||
|
|
||
| const currentMethodHasParameters = 'parameters' in method; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Played with this section a bit. Didn't come up with anything better. Any ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what you have here makes sense. I can follow what's happened easily, which I appreciate 👍
1235045 to
5336a84
Compare
|
|
||
| for (const [index, event] of events.entries()) { | ||
| tags.push({ | ||
| tagName: 'fires', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fires instead of @event like we currently do because @event is non-standard and shown on hover as @fires by Lit Plugin anyway.
a63dd60 to
be33bd7
Compare
5e08a0c to
be5e04f
Compare
| // Private because it's only meant to be used by Dropdown. | ||
| @property({ attribute: 'private-size', reflect: true }) | ||
| privateSize: 'small' | 'large' = 'large'; | ||
| privateSize: 'large' | 'small' = 'large'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made the order the same across components.
| * @attr {0.19.1} [version] | ||
| * | ||
| * @slot {Element | string} - The content of the accordion | ||
| * @slot {Element} [prefix-icon] - An icon before the label |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional slots, like optional attributes, use bracket notation. Lit Analyzer doesn't seem displeased by this.
|
@clintcs - I rebased one of my PRs off of this, and am having some issues. Locally, I'm seeing: I'm guessing that's causing the auto-generation to be messed up, as I'm now seeing lint errors after pushing: https://github.com/CrowdStrike/glide-core/actions/runs/13421939430/job/37496469111?pr=683 |
You definitely ran
Did you by chance bork the rebase? Because those lint errors look legitimate to me. |
Yup, I ran The lint errors may be legitimate, but I'm not seeing them in VS Code. |
Not sure what was going on. But the types are installed and correct. I pulled your branch. The errors went away for me after I restarted VS Code. |
I take it back. Still seeing a couple. I wonder why I wasn't seeing them on branch. Stand by! |
|
@clintcs - For the |
I wish there were. Unfortunately, the innermost quasi is the closest we can get because the markup isn't part of the ESLint AST. I found that super annoying as well. |
Still not seeing the issue on my branch. I'll keep digging. |
You nailed it. It's because the types are based off what's in |
danwenzel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome! After restarting VS Code, I saw and fixed the issues. Pretty smooth and straightforward.
Nice work! 🎉
Thank you! Good deal. I'm looking forward to seeing what else we can do with the manifest. |
|
Today got away from me. I'll pull this branch and review first thing tomorrow AM 👍 |
ynotdraw
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is excellent! Thanks a bunch for doing all of this, I know it was a lot. But this is really great work.
One nit I noticed: for the slot comments in particular there appears to be some trailing whitespace.
<!--
- An icon before the label
+ An icon before the label
@type {Element}
-->When I was testing things out, I hit save after some edits in render() and noticed the diff.
| "start:development:cem-analyze:comment": "JSON module imports produce a warning. Remove NODE_OPTIONS when JSON module imports are no longer experimental.", | ||
| "start:development:storybook": "storybook dev --config-dir .storybook --no-open --no-version-updates --port 6006", | ||
| "start:development:ts-morph": "chokidar ./custom-elements.json './src/ts-morph/**' --initial --silent --command 'tsx ./src/ts-morph/run.ts'", | ||
| "start:development:ts-morph:comment:tsx": "Remove `tsx` once Node.js type stripping is available.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
| "stylelint-use-nesting": "^6.0.0", | ||
| "terser": "^5.37.0", | ||
| "ts-lit-plugin": "^2.0.2", | ||
| "ts-morph": "^25.0.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow that's a lot of usage!
| // Checking if a component implements `FormControl` is the best proxy | ||
| // for whether it dispatches "invalid" events. Because simply checking | ||
| // for a static `formAssociated` property would wrongly include components | ||
| // like Button. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, nice job thinking through this one. Makes sense to me 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet! I feel like @required and FormControls came along just in time.
| return; | ||
| } | ||
|
|
||
| const currentMethodHasParameters = 'parameters' in method; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what you have here makes sense. I can follow what's happened easily, which I appreciate 👍
| // A hard-coded list is unfortunate. But we can't ignore all static | ||
| // properties because some may be meant for public use. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah... I feel this. I've got something similar going on with the Figma syncing stuff. Don't love it, but there's really not any other way. This approach looks good to me 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. It is what it is.
| ...getCssPropertyTags(declaration), | ||
| ...getEventTags(declaration), | ||
| ...getPropertyTags(declaration), | ||
| ...getMethodTags(declaration), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so, yeah. Your comment above resonates well with me on the reasoning 👍
| // Then `start:development:ts-morph` would run and write the component's | ||
| // file again, which would cause `start:development:cem-analyze` to run | ||
| // again. | ||
| if (isSave) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hah! Reminds me of when useEffect first came out with React and I kept finding myself in a similar situation. Sometimes DDoSing the server. Good times! 😅
Future us will appreciate this comment!
Thank you! Take a look? |
Looks good to me! Thank ya! |
|
@ynotdraw: I didn't run into any issues with Lit Analyzer after adding the JSDoc comments. But just a heads up given you're on the front line. It's possible consumers will report some issues if they're using Lit Analyzer. |
|
@clintcs sounds good, thanks for the heads up! I'll follow up with you if issues are reported 🫡 |
* Bump eslint-config-prettier from 9.1.0 to 10.0.1 (#639) Bumps [eslint-config-prettier](https://github.com/prettier/eslint-config-prettier) from 9.1.0 to 10.0.1. - [Release notes](https://github.com/prettier/eslint-config-prettier/releases) - [Changelog](https://github.com/prettier/eslint-config-prettier/blob/main/CHANGELOG.md) - [Commits](prettier/eslint-config-prettier@v9.1.0...v10.0.1) --- updated-dependencies: - dependency-name: eslint-config-prettier dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump typescript from 5.7.2 to 5.7.3 (#640) Bumps [typescript](https://github.com/microsoft/TypeScript) from 5.7.2 to 5.7.3. - [Release notes](https://github.com/microsoft/TypeScript/releases) - [Changelog](https://github.com/microsoft/TypeScript/blob/main/azure-pipelines.release.yml) - [Commits](microsoft/TypeScript@v5.7.2...v5.7.3) --- updated-dependencies: - dependency-name: typescript dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump @storybook/theming from 8.5.1 to 8.5.2 (#641) Bumps [@storybook/theming](https://github.com/storybookjs/storybook/tree/HEAD/code/lib/theming) from 8.5.1 to 8.5.2. - [Release notes](https://github.com/storybookjs/storybook/releases) - [Commits](https://github.com/storybookjs/storybook/commits/v8.5.2/code/lib/theming) --- updated-dependencies: - dependency-name: "@storybook/theming" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump vite from 6.0.9 to 6.0.11 (#643) Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 6.0.9 to 6.0.11. - [Release notes](https://github.com/vitejs/vite/releases) - [Changelog](https://github.com/vitejs/vite/blob/main/packages/vite/CHANGELOG.md) - [Commits](https://github.com/vitejs/vite/commits/v6.0.11/packages/vite) --- updated-dependencies: - dependency-name: vite dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add Resize Observer directive (#642) * Bump vitest from 2.1.8 to 3.0.4 (#644) Bumps [vitest](https://github.com/vitest-dev/vitest/tree/HEAD/packages/vitest) from 2.1.8 to 3.0.4. - [Release notes](https://github.com/vitest-dev/vitest/releases) - [Commits](https://github.com/vitest-dev/vitest/commits/v3.0.4/packages/vitest) --- updated-dependencies: - dependency-name: vitest dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add `@required` decorator (#638) * Add `@final` decorator (#645) * Give Input a minimum width (#647) * Use `@final` decorator everywhere (#648) * Add Label `label` attribute (#646) * Move test-only rules to their own block (#652) * Prevent form control overflow (#650) * Add logo `width` attribute * Make Dropdown's `placeholder` attribute optional (#657) * Added "add" Dropdown event documentation (#660) * Drawer no longer closes on Escape press (#667) * Drawer no longer closing on Esc * Update .changeset/tiny-pens-drop.md Co-authored-by: Dan Wenzel <[email protected]> * Update .changeset/tiny-pens-drop.md Co-authored-by: clintcs <[email protected]> --------- Co-authored-by: Dan Wenzel <[email protected]> Co-authored-by: clintcs <[email protected]> * Add JSDoc comments for multi-word property names (#661) * Upgrade Storybook (#669) * Added severity icon support to Modal (#668) * Added severity icon support to Modal * Apply suggestions from code review Co-authored-by: clintcs <[email protected]> --------- Co-authored-by: clintcs <[email protected]> * Bump lint-staged from 15.2.11 to 15.4.3 (#666) Bumps [lint-staged](https://github.com/lint-staged/lint-staged) from 15.2.11 to 15.4.3. - [Release notes](https://github.com/lint-staged/lint-staged/releases) - [Changelog](https://github.com/lint-staged/lint-staged/blob/master/CHANGELOG.md) - [Commits](lint-staged/lint-staged@v15.2.11...v15.4.3) --- updated-dependencies: - dependency-name: lint-staged dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump rimraf from 5.0.10 to 6.0.1 (#665) Bumps [rimraf](https://github.com/isaacs/rimraf) from 5.0.10 to 6.0.1. - [Changelog](https://github.com/isaacs/rimraf/blob/main/CHANGELOG.md) - [Commits](isaacs/rimraf@v5.0.10...v6.0.1) --- updated-dependencies: - dependency-name: rimraf dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump chalk from 5.3.0 to 5.4.1 (#664) Bumps [chalk](https://github.com/chalk/chalk) from 5.3.0 to 5.4.1. - [Release notes](https://github.com/chalk/chalk/releases) - [Commits](chalk/chalk@v5.3.0...v5.4.1) --- updated-dependencies: - dependency-name: chalk dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump postcss from 8.4.49 to 8.5.1 (#663) Bumps [postcss](https://github.com/postcss/postcss) from 8.4.49 to 8.5.1. - [Release notes](https://github.com/postcss/postcss/releases) - [Changelog](https://github.com/postcss/postcss/blob/main/CHANGELOG.md) - [Commits](postcss/postcss@8.4.49...8.5.1) --- updated-dependencies: - dependency-name: postcss dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump @typescript-eslint/rule-tester from 8.19.0 to 8.22.0 (#662) * Bump @typescript-eslint/rule-tester from 8.19.0 to 8.22.0 Bumps [@typescript-eslint/rule-tester](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/rule-tester) from 8.19.0 to 8.22.0. - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/rule-tester/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.22.0/packages/rule-tester) --- updated-dependencies: - dependency-name: "@typescript-eslint/rule-tester" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Bump @typescript-eslint and related packages --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: clintcs <[email protected]> * Bump vitest from 3.0.4 to 3.0.5 (#676) Bumps [vitest](https://github.com/vitest-dev/vitest/tree/HEAD/packages/vitest) from 3.0.4 to 3.0.5. - [Release notes](https://github.com/vitest-dev/vitest/releases) - [Commits](https://github.com/vitest-dev/vitest/commits/v3.0.5/packages/vitest) --- updated-dependencies: - dependency-name: vitest dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix Popover `<label>` bug (#672) * Radio Group bugfixes (#670) * Fix Tooltip overflow bug (#673) * Clean up tests * Update README and CONTRIBUTING (#684) * Update README and CONTRIBUTING * Update CONTRIBUTING.md Co-authored-by: clintcs <[email protected]> * Update CONTRIBUTING.md Co-authored-by: clintcs <[email protected]> --------- Co-authored-by: clintcs <[email protected]> * Bump esbuild from 0.24.2 to 0.25.0 (#687) Bumps [esbuild](https://github.com/evanw/esbuild) from 0.24.2 to 0.25.0. - [Release notes](https://github.com/evanw/esbuild/releases) - [Changelog](https://github.com/evanw/esbuild/blob/main/CHANGELOG-2024.md) - [Commits](evanw/esbuild@v0.24.2...v0.25.0) --- updated-dependencies: - dependency-name: esbuild dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Tree Item: Prevent prefix icon from shrinking (#686) Wraps the prefix icon slot in a flex container, to prevent slotted SVGs from shrinking at narrow widths * Make use of the new `@required` decorator (#685) * Make use of the new required decorator * Apply suggestions from code review Co-authored-by: clintcs <[email protected]> * Remove unnecessary this.selectedOptions.length > 0 checks * Update CONTRIBUTING.md Co-authored-by: Dan Wenzel <[email protected]> --------- Co-authored-by: clintcs <[email protected]> Co-authored-by: Dan Wenzel <[email protected]> * Remove gap between Checkbox or Toggle and summary, if there is no summary (#688) * Remove gap between Checkbox or Toggle and summary if there is no summary * Update .changeset/cool-hornets-talk.md Co-authored-by: clintcs <[email protected]> --------- Co-authored-by: clintcs <[email protected]> * Add `type="time"` support to Input (#690) * Patch dependencies (#693) * Prepare for the elements manifest (#691) * Clear single-select Dropdown's input field when `value` is emptied (#694) * Add elements manifest and JSDoc generation (#692) * Add a minimum width to filterable Dropdown's input field (#696) * Update Dropdown test * Fix Modal test lint issue * Bump component version in manifest and JSDoc comments --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Tony Ward <[email protected]> Co-authored-by: Dan Wenzel <[email protected]>


🚀 Description
@custom-elements-manifest/analyzer. The rest comes from the analyzer plugins I've added.ts-morphand some code transformations. The transformations use what's in manifest to add JSDoc comments to each component. Comments includes attributes, slots, custom properties, events, properties, and methods.📋 Checklist
🔬 How to Test
Review each component's JSDoc comment. If it's accurate and well-formatted, then the manifest is correct and the transforms are doing their job. Given all that's involved, I'm sure I flubbed a thing or two.
It also wouldn't hurt to pull this branch, install the new dependencies, run
pnpm start, and play around with adding attributes, properties, methods, and slots to components.📸 Images/Videos of Functionality
N/A