feat(create-cli): setup wizard architecture#1248
Conversation
|
View your CI Pipeline Execution ↗ for commit 875ff60
☁️ Nx Cloud last updated this comment at |
@code-pushup/ci
@code-pushup/cli
@code-pushup/core
@code-pushup/create-cli
@code-pushup/models
@code-pushup/nx-plugin
@code-pushup/axe-plugin
@code-pushup/eslint-plugin
@code-pushup/js-packages-plugin
@code-pushup/coverage-plugin
@code-pushup/jsdocs-plugin
@code-pushup/lighthouse-plugin
@code-pushup/typescript-plugin
@code-pushup/utils
commit: |
Code PushUp🤨 Code PushUp report has both improvements and regressions – compared current commit d1f33c6 with previous commit 4c80af5. 🕵️ See full comparison in Code PushUp portal 🔍 🏷️ Categories👍 3 groups improved, 👎 1 group regressed, 👍 9 audits improved, 👎 5 audits regressed, 14 audits changed without impacting score🗃️ Groups
30 other groups are unchanged. 🛡️ Audits
650 other audits are unchanged. |
Code PushUp🤨 Code PushUp report has both improvements and regressions – compared current commit d1f33c6 with previous commit 4c80af5. 💼 Project
|
| 🏷️ Category | ⭐ Previous score | ⭐ Current score | 🔄 Score change |
|---|---|---|---|
| Documentation | 🟡 64 | 🔴 19 | |
| Code coverage | 🟢 96 | 🟢 98 |
4 other categories are unchanged.
👍 1 group improved, 👎 1 group regressed, 👍 3 audits improved, 👎 3 audits regressed, 2 audits changed without impacting score
🗃️ Groups
| 🔌 Plugin | 🗃️ Group | ⭐ Previous score | ⭐ Current score | 🔄 Score change |
|---|---|---|---|---|
| JSDocs coverage | Documentation coverage | 🟡 64 | 🔴 19 | |
| Code coverage | Code coverage metrics | 🟢 96 | 🟢 98 |
13 other groups are unchanged.
🛡️ Audits
| 🔌 Plugin | 🛡️ Audit | 📏 Previous value | 📏 Current value | 🔄 Value change |
|---|---|---|---|---|
| JSDocs coverage | Methods coverage | 🟩 0 undocumented methods | 🟥 4 undocumented methods | |
| JSDocs coverage | Properties coverage | 🟩 0 undocumented properties | 🟥 2 undocumented properties | |
| JSDocs coverage | Classes coverage | 🟩 0 undocumented classes | 🟥 1 undocumented classes | |
| JSDocs coverage | Types coverage | 🟥 1 undocumented types | 🟥 12 undocumented types | |
| Code coverage | Branch coverage | 🟨 87.5 % | 🟩 93.2 % | |
| Code coverage | Line coverage | 🟩 97 % | 🟩 98.5 % | |
| JSDocs coverage | Functions coverage | 🟥 6 undocumented functions | 🟥 11 undocumented functions | |
| JSDocs coverage | Variables coverage | 🟥 5 undocumented variables | 🟥 3 undocumented variables |
435 other audits are unchanged.
💼 Project utils
🥳 Code PushUp report has improved.
🕵️ See full comparison in Code PushUp portal 🔍
| 🏷️ Category | ⭐ Previous score | ⭐ Current score | 🔄 Score change |
|---|---|---|---|
| Documentation | 🟡 60 | 🟡 60 |
5 other categories are unchanged.
👍 1 group improved
🗃️ Groups
| 🔌 Plugin | 🗃️ Group | ⭐ Previous score | ⭐ Current score | 🔄 Score change |
|---|---|---|---|---|
| JSDocs coverage | Documentation coverage | 🟡 60 | 🟡 60 |
14 other groups are unchanged.
🛡️ Audits
All of 444 audits are unchanged.
12 other projects are unchanged.
matejchalk
left a comment
There was a problem hiding this comment.
Looks really promising 🙂
| function buildExportStatement(plugins: PluginCodegenResult[]): string { | ||
| const items = plugins.map(({ pluginInit }) => pluginInit).join(', '); | ||
| return `export default { plugins: [${items}] } satisfies CoreConfig;`; | ||
| } |
There was a problem hiding this comment.
Have you considered using StatementStructures? It seems a bit more robust than concatenating strings. The plugins could also register their init code as an AST.
There was a problem hiding this comment.
I decided to drop the dependency on ts-morph. @BioPhoton is right to question this choice: it adds unnecessary complexity. I replaced it with a small CodeBuilder class that should suffice for the current and future use cases.
| [ | ||
| "import type { CoreConfig } from '@code-pushup/models';", | ||
| 'export default { plugins: [] } satisfies CoreConfig;', | ||
| '', | ||
| ].join('\n'), |
There was a problem hiding this comment.
It would be more readable to have a separator line between the imports and the config export. 🙏
| expect(generateConfigSource(plugins)).toBe( | ||
| [ | ||
| "import type { CoreConfig } from '@code-pushup/models';", | ||
| "import eslintPlugin from '@code-pushup/eslint-plugin';", | ||
| "import coveragePlugin from '@code-pushup/coverage-plugin';", | ||
| "export default { plugins: [await eslintPlugin(), await coveragePlugin({ reports: [{ resultsPath: 'coverage/lcov.info', pathToProject: '' }] })] } satisfies CoreConfig;", | ||
| '', | ||
| ].join('\n'), | ||
| ); |
There was a problem hiding this comment.
This generates quite a long line. Does ts-morph not wrap long lines? If not, we could run prettier on the output.
|
As a nit pick question I want to understand if we could choose another library that ts-morph to achieve the same goal. The library is old and slow and I thought nowadays must be a more performant lib on the market. What I can say about ts-morph is, Instead we could write a small typed builder: generateConfig({
imports: [...],
plugins: [...],
categories: [...]
})Or use @swc/core AST builder: |
Closes #1240
Break
create-cli's dependency onnx-pluginand rebuild the package around a modular setup wizard. Usests-morphfor AST-based config generation,@inquirer/promptsfor interactive input, and a virtualTreefor buffered file writes with dry-run support.