-
Notifications
You must be signed in to change notification settings - Fork 23
Piyush/sdk overhaul #133
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?
Piyush/sdk overhaul #133
Conversation
…gs and environment files.
…for image and video handling, configuration files, and a test application. Update .gitignore and add .npmignore for build artifacts and dependencies.
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 represents a major SDK overhaul for the ImageKit Angular library, migrating from an older Angular architecture to a modern, standalone component-based approach compatible with Angular 12-20 LTS.
Key Changes:
- Complete rewrite using standalone components with SSR support
- Migration from
imagekitio-angularto@imagekit/angularpackage name - Removal of legacy code, tests, and deprecated configuration files
Reviewed changes
Copilot reviewed 79 out of 90 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| verify-build.js | New build verification script to validate package structure |
| test-app/* | New test application demonstrating SDK usage with Angular 17+ |
| projects/imagekit-angular/* | New SDK implementation with standalone components, services, and modern TypeScript |
| sdk/* | Removal of legacy SDK files, tests, and configurations |
| package.json | New workspace configuration for Angular 17 |
| angular.json | New Angular workspace configuration |
Files not reviewed (1)
- sdk/tests/test-apps/sample-server/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1 @@ | |||
| <!-- Placeholder favicon --> | |||
Copilot
AI
Dec 29, 2025
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.
The favicon.ico file contains an HTML comment instead of actual favicon binary data. A .ico file should contain binary image data, not HTML comments. This will cause browsers to fail loading the favicon.
| <!-- Placeholder favicon --> |
| * ``` | ||
| */ | ||
| async upload(options: UploadOptions): Promise<UploadResponse> { | ||
| const mergedOptions: any = { |
Copilot
AI
Dec 29, 2025
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.
The mergedOptions variable is typed as any, which defeats the purpose of TypeScript's type safety. It should be typed as UploadOptions or a more specific interface that extends it.
| const mergedOptions: any = { | |
| const mergedOptions: UploadOptions = { |
… for mergedOptions, enhancing type safety and clarity.
No description provided.