-
Notifications
You must be signed in to change notification settings - Fork 22
build: standardize tsconfig to Node16 and fix build inconsistencies #422
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
|
|
@revaarathore11 is attempting to deploy a commit to the elixir-cloud-aai Team on Vercel. A member of the Team first needs to authorize it. |
Reviewer's GuideCentralizes TypeScript configuration across all packages by introducing a shared tsconfig.base.json with Node16 settings, updating package and template tsconfig.json files to extend it, and fixing an import path in the cloud registry client to work with the new build output structure. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `packages/ecc-client-elixir-cloud-registry/src/providers/cr-provider.ts:14` </location>
<code_context>
ExternalService,
Error,
-} from "@elixir-cloud/service-registry/dist/providers";
+} from "@elixir-cloud/service-registry/providers";
// Re-export base types for use in other modules
</code_context>
<issue_to_address>
**issue (bug_risk):** Switching from a dist deep import to a bare subpath may break resolution if the package doesn’t expose that path via exports.
This change from "@elixir-cloud/service-registry/dist/providers" to "@elixir-cloud/service-registry/providers" is nicer, but only if the package exposes a `./providers` subpath (via `exports` in `package.json` or a compatible on-disk layout). If it isn’t explicitly exported, TS path mapping might succeed while Node/bundlers fail at runtime. Please confirm that `@elixir-cloud/service-registry/providers` is an officially supported entrypoint.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@anuragxxd please review |
Description
The monorepo contained inconsistent TypeScript build configurations across its 10 packages. Some packages used modern Node16 settings while others used legacy esnext settings. This inconsistency led to different output structures in dist/ folders, causing potential import issues and type definition problems for consumers.
Fixes #416
I centralized the TypeScript configuration by introducing a
tsconfig.base.json
that enforces module: Node16 and moduleResolution: node16. This ensures that all packages, regardless of their individual quirks, build with the same rules. I also fixed specific codebase issues (like the /dist/ import path in cloud-registry) that were blocking the build under this standardized configuration
Checklist
Comments
Summary by Sourcery
Standardize TypeScript configuration across packages using a shared base config and align imports with the new build output structure.
Enhancements:
Build: