chore(monorepo): break out generators#778
chore(monorepo): break out generators#778avivkeller wants to merge 3 commits intofeature/monorepofrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryHigh Risk Overview Updates tooling and CI to use the new generator specifiers (workflows, docs/examples, eslint/prettier/c8 ignores), and trims Refactors shared legacy HTML/JSON utilities into Reviewed by Cursor Bugbot for commit 0a923b4. Bugbot is set up for automated code reviews on this repo. Configure here. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feature/monorepo #778 +/- ##
===================================================
Coverage ? 74.29%
===================================================
Files ? 148
Lines ? 14175
Branches ? 1104
===================================================
Hits ? 10532
Misses ? 3637
Partials ? 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| "unist-util-select": "^5.1.0", | ||
| "unist-util-visit": "^5.1.0", | ||
| "yaml": "^2.8.3" | ||
| } |
There was a problem hiding this comment.
Missing tinyglobby and glob-parent dependencies in internal package
High Severity
tinyglobby is imported in both ast/index.mjs and ast-js/index.mjs, and glob-parent is imported in ast/index.mjs, but neither is declared in @doc-kittens/internal's package.json dependencies. Both were removed from @node-core/doc-kit (core) during this refactor. This will cause runtime MODULE_NOT_FOUND errors when the generators are invoked.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 2240b47. Configure here.
|
@avivkeller can you disable automatic copilot reviews in your acc? 👀 |
Can we call it @node-core/doc-kit-legacy-html, @node-core/doc-kit-legacy-json, since eventually they will be removed and they are node-core only.
I think this must be split. IMO jsx-ast should be part of the core of doc-kit, not in a separate package. orama-db should be @doc-kittens/orama-db... I don't know what the web one should be since it is right now Node.js-specific by default and probably in the future should use neutral colours and a neutral logo by default without any Node.js-specific theming. And if this is the react one, it should probably be called @doc-kittens/react.
Should be @doc-kittens/sitemap, @doc-kittens/llms
IMO, these should be shipped with doc-kit, not in an extra package.
What would be everything else? Examples? |
|
The idea behind the monorepo was to avoid generators importing packages and files they don't actually need. For legacy-html, for instance, it depends on some utilities in legacy-json, so it makes sense to put them in the same package. The extras package isn't a permanent name, it's merely holding everything not already in a package for now. I'll probably break those out into individual packages. |
I disagree, in the event that a generator doesn't want that data, and merely wants to create its own AST generator and whatnot, there's no need for it to require these |
-1, the current name is IMO fine. The scope doesn't really matter. |
-1 on your -1, the scope does matter. |
The core generators should be part of doc-kit, it doesn't make sense that even the basic ones such as ast, metadata, jsx-ast, etc are in individual packages, they should be shipped with doc-kit, and not be even more packages. |
Then said utilities should not be shared between them and be part of doc-kit itself. Except the legacy ones, all legacy generators should be their own package.
Can you still please describe what's inside extra? It doesn't make sense at all having a package called "extra" how are people going to know what's inside extra? The idea is that each of the non-core generators is their own isolated package, so that users can install the generators they want. These packages shouldn't be a set of generators. Think of how Radix UI does this. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0a923b4. Configure here.
|
|
||
| # Templates | ||
| packages/core/src/generators/web/template.html | ||
| packages/legacy/src/generators/web/template.html |
There was a problem hiding this comment.
.prettierignore points to wrong package for web template
Low Severity
The web generator template was moved to packages/react/src/generators/web/template.html, but .prettierignore incorrectly references packages/legacy/src/generators/web/template.html. The legacy package has html and json generators, not web. This means Prettier won't ignore the actual template file in the react package.
Reviewed by Cursor Bugbot for commit 0a923b4. Configure here.
| "packages/core/src/generators/legacy-html/assets", | ||
| "packages/core/src/generators/web/ui", | ||
| "packages/legacy/src/generators/html/assets", | ||
| "packages/react/src/generators/web/ui", |
There was a problem hiding this comment.
.c8rc.json excludes wrong path for web UI
Low Severity
The coverage exclusion path packages/react/src/generators/web/ui is incorrect. The eslint config in the same PR references the web UI at packages/react/src/utils/web/ui/**/*, and the web generator imports ROOT from ../../utils/web/constants.mjs then resolves UI components relative to that. The UI files live under utils/web/ui, not generators/web/ui.
Reviewed by Cursor Bugbot for commit 0a923b4. Configure here.
|
Forgot to post this before: Also to clarify, I just want to understand the process of thought, because I thought that the idea I presented originally was quite clear and thought the execution was also clear 😅 |
This PR breaks out the monorepo feature branch into:
@doc-kittens/legacy(legacy-*)@doc-kittens/react(orama-db,webTBRreact-html,jsx-ast)@doc-kittens/web(sitemap,llms-txt)@doc-kittens/internal(ast-*,metadata)@doc-kittens/extras(Everything else)