-
Notifications
You must be signed in to change notification settings - Fork 14
refactor: api journeys workspace #8630
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: main
Are you sure you want to change the base?
Conversation
- Adjusted .gitignore to correctly include node_modules. - Updated nx.json to specify pnpm as the package manager. - Removed deprecated NestJS dependencies from package.json. - Updated pnpm-lock.yaml to reflect changes in dependencies. - Added new packages to pnpm-workspace.yaml. - Refactored TypeScript paths in tsconfig.base.json for better module resolution. - Updated project.json paths in api-journeys for TranslationModule. - Removed unused NestJS common library and its related files. - Refactored imports in various files to align with the new structure.
- Updated various dependencies in `package.json` for `@nestjs` packages and others to their latest versions. - Removed the unused webpack IgnorePlugin configuration from `webpack.config.js` as it was no longer necessary.
WalkthroughUpdates repository workspace config and the api-journeys service: broadens Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 5502002
☁️ Nx Cloud last updated this comment at |
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apis/api-journeys/tsconfig.json`:
- Around line 10-13: The path aliases in apis/api-journeys/tsconfig.json for
"@core/nest/common/*", "@core/nest/decorators/*", "@core/nest/gqlAuthGuard/*",
and "@core/nest/powerBi/*" incorrectly use "./libs/..." (resolving inside
apis/api-journeys); change each mapping to point at the repo-root libs by
replacing "./libs/..." with "../../libs/..." so they resolve to
"../../libs/nest/<package>/src/lib/*" (update all four alias entries
accordingly).
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
I see you added the "on stage" label, I'll get this merged to the stage branch! |
- Updated the command in project.json to point to the new location of the generate-typings script. - Deleted the outdated generate-typings.ts script as it is no longer needed.
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apis/api-journeys/scripts/generate-typings.ts`:
- Around line 8-12: The typePaths/path entries currently build locations with
process.cwd() which makes the script sensitive to the working directory; change
them to resolve relative to the script file (use __dirname or path.resolve with
join from the script directory) so typePaths and the output path (the variables
referenced as typePaths and path in this file) point to 'src/app/**/*.graphql'
and 'src/app/__generated__/graphql.ts' relative to the script location rather
than process.cwd(); update any uses of join(process.cwd(), ...) to
join(__dirname, ...) or path.resolve(__dirname, '..', 'src', ...) accordingly so
the script runs correctly from any working directory.
| typePaths: [join(process.cwd(), 'apis/api-journeys/src/app/**/*.graphql')], | ||
| path: join( | ||
| process.cwd(), | ||
| `apis/${process.argv[2]}/src/app/__generated__/graphql.ts` | ||
| 'apis/api-journeys/src/app/__generated__/graphql.ts' | ||
| ), |
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.
Avoid process.cwd()-dependent paths.
If this script is run from apis/api-journeys/, the current process.cwd() usage will double the path (e.g., apis/api-journeys/apis/api-journeys/...). Resolve paths relative to the script instead to make execution location-agnostic.
🔧 Suggested adjustment
-import { join } from 'path'
+import { join, resolve } from 'path'
const definitionsFactory = new GraphQLFederationDefinitionsFactory()
+const apiJourneysRoot = resolve(__dirname, '..')
definitionsFactory
.generate({
- typePaths: [join(process.cwd(), 'apis/api-journeys/src/app/**/*.graphql')],
+ typePaths: [join(apiJourneysRoot, 'src/app/**/*.graphql')],
path: join(
- process.cwd(),
- 'apis/api-journeys/src/app/__generated__/graphql.ts'
+ apiJourneysRoot,
+ 'src/app/__generated__/graphql.ts'
),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| typePaths: [join(process.cwd(), 'apis/api-journeys/src/app/**/*.graphql')], | |
| path: join( | |
| process.cwd(), | |
| `apis/${process.argv[2]}/src/app/__generated__/graphql.ts` | |
| 'apis/api-journeys/src/app/__generated__/graphql.ts' | |
| ), | |
| import { join, resolve } from 'path' | |
| const definitionsFactory = new GraphQLFederationDefinitionsFactory() | |
| const apiJourneysRoot = resolve(__dirname, '..') | |
| definitionsFactory | |
| .generate({ | |
| typePaths: [join(apiJourneysRoot, 'src/app/**/*.graphql')], | |
| path: join( | |
| apiJourneysRoot, | |
| 'src/app/__generated__/graphql.ts' | |
| ), |
🤖 Prompt for AI Agents
In `@apis/api-journeys/scripts/generate-typings.ts` around lines 8 - 12, The
typePaths/path entries currently build locations with process.cwd() which makes
the script sensitive to the working directory; change them to resolve relative
to the script file (use __dirname or path.resolve with join from the script
directory) so typePaths and the output path (the variables referenced as
typePaths and path in this file) point to 'src/app/**/*.graphql' and
'src/app/__generated__/graphql.ts' relative to the script location rather than
process.cwd(); update any uses of join(process.cwd(), ...) to join(__dirname,
...) or path.resolve(__dirname, '..', 'src', ...) accordingly so the script runs
correctly from any working directory.
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.
What packages do we need to use this for? If it's only a few we should probably use public hoist pattern.
public-hoist-pattern[]=eslint
public-hoist-pattern[]=@types/*
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.