-
Notifications
You must be signed in to change notification settings - Fork 0
fix(create): added missing deps and added test for workspace generator #97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
View your CI Pipeline Execution ↗ for commit 05fa095
☁️ Nx Cloud last updated this comment at |
…ps in create gene workspace process
…storybook and custom storybook targets
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.
Pull Request Overview
This PR fixes missing dependencies, refactors generator directory handling, and adds a test for the workspace generator.
- Use
getWorkspaceLayout
to derivelibsDir
for several generators, removing hardcoded paths. - Simplify Storybook webpack configuration and handle SCSS loading.
- Add an integration test for
gene-workspace-generator
and bump package versions with new devDependencies.
Reviewed Changes
Copilot reviewed 14 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
packages/gene-tools/src/generators/translations-library-generator/index.ts | Inject libsDir for dynamic directory in translations library |
packages/gene-tools/src/generators/storybook-init/files/main.js__tmpl__ | Simplified webpackFinal , updated SCSS rule, removed old loaders |
packages/gene-tools/src/generators/storybook-configuration/utilities/add-storybook-targets.ts | Updated Storybook target names and dependencies ordering |
packages/gene-tools/src/generators/storybook-configuration/snapshots/index.spec.ts.snap | Updated snapshots to reflect new Storybook target configurations |
packages/gene-tools/src/generators/module-generator/index.ts | Added getWorkspaceLayout to compute e2eTestingProvidersPath |
packages/gene-tools/src/generators/module-generator/files/module/fileName/pascalCaseFileName.stories.tsx__tmpl__ | Updated StorybookProviders import to use computed path |
packages/gene-tools/src/generators/library-generator/index.ts | Adjusted baseDirectory logic |
packages/gene-tools/src/generators/gene-workspace-generator/index.ts | Wrapped generator calls in try/catch , added logging |
packages/gene-tools/src/generators/gene-workspace-generator/index.spec.ts | Added test and debug logs for workspace generator |
packages/gene-tools/src/generators/e2e-providers-generator/index.ts | Mirror directory refactoring via getWorkspaceLayout |
packages/gene-tools/src/generators/assets-library-generator/index.ts | Mirror directory refactoring via getWorkspaceLayout |
packages/gene-tools/src/executors/storybook/storybook.ts | Updated default targets to storybook /build-storybook |
package.json | Bumped version, added ajv , ajv-keywords , @nx/linter dependencies |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (5)
packages/gene-tools/src/generators/gene-workspace-generator/index.ts:24
- [nitpick] The catch block logs a generic message for any error in the try, including translation and file generation steps. Consider narrowing the catch to only asset creation or updating the message to reflect which operation failed.
console.error('Error creating assets library:', error);
packages/gene-tools/src/generators/storybook-init/files/main.js__tmpl__:65
- The custom webpackFinal override removes rules for handling TypeScript files and SVGs, which will break TSX and asset loading in Storybook. You should reintroduce the ts-loader rule and inline SVG loader after filtering defaults.
webpackFinal: async (config) => {
packages/gene-tools/src/generators/module-generator/files/module/fileName/pascalCaseFileName.stories.tsx__tmpl__:6
- Including
libsDir
in the import path makes the package name incorrect. The library package should be imported by its npm scope and package name (e.g.@scope/e2e-testing-providers
), not including thelibs
folder.
import {StorybookProviders} from '@<%= e2eTestingProvidersPath %>';
packages/gene-tools/src/generators/library-generator/index.ts:19
- Changing
baseDirectory
to remove the trailing slash may break path concatenation in downstream templates. The original logic ensured a separator was present.
const baseDirectory = schema.directory || '';
packages/gene-tools/src/generators/gene-workspace-generator/index.spec.ts:24
- [nitpick] The test contains console.log statements for debugging, which can clutter CI logs. It’s better to rely on assertions and remove these logs before merging.
console.log('🔍 Before running generator:');
No description provided.