-
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.
… for mergedOptions, enhancing type safety and clarity.
- Introduced Apache License 2.0 in the LICENSE file. - Updated README.md to reflect the new package name, installation instructions, and improved component usage examples for the ImageKit Angular SDK. - Removed outdated sections and clarified the configuration and component details for better user guidance.
- Introduced a new BindDirective for applying dynamic attributes to elements. - Updated IKImageComponent and IKVideoComponent to utilize the BindDirective for passthrough attributes. - Enhanced IKImageProps and IKVideoProps interfaces to include passthrough attributes. - Updated public API to export the new BindDirective. - Added a new UploadTestComponent for testing image uploads with ImageKit service.
… tab state - Added UploadTestComponent to the imports of AppComponent for image upload functionality. - Introduced activeTab property to manage the current tab state between 'display' and 'upload'. - Implemented setActiveTab method to update the active tab based on user interaction.
…ld and type safety - Bump version to 0.0.1 in package.json. - Modify build script to use production configuration. - Add npm pack script for packaging the library. - Enhance TypeScript configuration with strict template checks and input access modifiers in tsconfig.lib.json and tsconfig.lib.prod.json.
…tails - Added Node.js engine requirement to package.json. - Modified build script to remove production configuration for flexibility. - Updated npm pack script to ensure build completion before packaging. - Enhanced README with new section on build output structure and local configuration examples for better user guidance.
…for URL validation and transformation configuration
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
Copilot reviewed 85 out of 96 changed files in this pull request and generated 1 comment.
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.
| `] | ||
| }) | ||
| export class UploadTestComponent { | ||
| authenticationType: 'authenticationServer' | 'signature' = 'authenticationServer'; |
Copilot
AI
Jan 2, 2026
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 property authenticationType should follow Angular naming conventions by being prefixed with public or private access modifier for better code clarity and consistency.
| authenticationType: 'authenticationServer' | 'signature' = 'authenticationServer'; | |
| public authenticationType: 'authenticationServer' | 'signature' = 'authenticationServer'; |
.github/workflows/nodejs.yml
Outdated
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.
We would need this workflow.
.github/workflows/npmpublish.yml
Outdated
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.
This file is needed
- Refactor .gitignore to include Playwright test results and reports. - Update package-lock.json and package.json to reflect version changes and add Playwright dependencies. - Introduce Playwright configuration and testing scripts in test-app. - Add comprehensive end-to-end tests for IKImage and IKVideo components, covering rendering, transformations, and responsive behavior. - Create integration and transformation tests to validate component functionality and URL handling. - Enhance app.component.html with updated attributes for testing purposes.
- Remove explicit Playwright browser installation step, now handled by postinstall script. - Update test command to use npm script for running Playwright tests. - Add list reporter configuration for real-time output in Playwright.
…ywright server command
…setup - Set "skipLibCheck" to false in tsconfig.json for stricter type checking. - Simplify e2e workflow by limiting the project matrix to "Desktop Chrome" only. - Increase timeout settings in Playwright configuration for tests and assertions. - Adjust Playwright test command to remove unnecessary project specification.
fb40aeb to
7803a2a
Compare
… for improved build and testing
…d packages for compatibility
…22.0.0 and update package version to 6.0.0
| * }); | ||
| * ``` | ||
| */ | ||
| buildSrc(options: SrcOptions): string { |
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.
ImageKitService wraps stateless utility functions solely to auto-merge config (urlEndpoint, publicKey). However, users calling these functions directly are doing advanced/custom work and should explicitly provide config anyway. Since <ik-image> and <ik-video> components handle 95% of use cases, recommend removing the service and keeping simple re-exports for better simplicity, smaller bundles, and framework-agnostic utilities.
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.
will remove the imagekit service from public-api exports
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.
other functions are being exported from @imagekit/js already
projects/imagekit-angular/src/lib/components/ik-video.component.ts
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| ngOnChanges(changes: SimpleChanges): void { | ||
| this.updateImageAttributes(); |
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.
Runs even if only className changed.
Should check: if (changes['src'] || changes['transformation'] || changes['urlEndpoint'])?
basically our props are changed. Verify it.
| } | ||
| } | ||
|
|
||
| ngAfterViewInit(): void { |
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.
Are we calling Passthrough twice?
ngOnChanges() {
if (changes['passthrough']) this.bindDirective.setAttrs(...)
}
ngAfterViewInit() {
if (this.passthrough) this.bindDirective.setAttrs(...) // Duplicate!
}
projects/imagekit-angular/src/lib/components/ik-image.component.ts
Outdated
Show resolved
Hide resolved
| */ | ||
| @Component({ | ||
| selector: 'ik-image', | ||
| standalone: true, |
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.
Should we add exportAs: 'ikImage' and in videos too?
AI says, Benefits for DX:
- Debugging - Users can inspect generated URLs in templates
- Testing - Can access component state in E2E tests
- Advanced Control - Parent can trigger component methods
- Transparency - Makes component behavior more observable
Verify and do the neeful.
| * provideImageKit({ | ||
| * urlEndpoint: 'https://ik.imagekit.io/your_imagekit_id', | ||
| * publicKey: 'your_public_key', | ||
| * authenticationEndpoint: 'https://your-server.com/auth' |
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 is authenticationEndpoint even doing? This whole things looks outdated and incorrect.
None of our sdk now needs authenticationEndpoint as upload function expects the signature, token and publicKey directly.
Same for publicKey. SDK does't need it now.
…mponent for better null handling
| /** | ||
| * Your ImageKit public key (required for upload functionality) | ||
| */ | ||
| publicKey?: string; |
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.
Get rid of publicKey and authenticationEndpoint
| /** | ||
| * Get your urlEndpoint from the ImageKit dashboard (https://imagekit.io/dashboard/url-endpoints). | ||
| * | ||
| * You can also set `urlEndpoint` globally using `provideImageKit()` or `IMAGEKIT_CONFIG` token, |
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.
Mentions IMAGEKIT_CONFIG token which is implementation detail. Users should only know about provideImageKit().
| * | ||
| * @example | ||
| * ```typescript | ||
| * passthrough={{ 'data-testid': 'my-image', 'aria-label': 'Hero image' }} |
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.
Example uses JSX syntax {{}} but this is Angular. Should use [passthrough] binding.
…plement OnDestroy lifecycle in BindDirective
…onfiguration and simplify upload options handling
…necessary tracking
…perty types in IKImageProps and IKVideoProps for ngStyle compatibility
| sizes="(max-width: 600px) 100vw, 50vw" | ||
| ></ik-image> | ||
| a | ||
| <ik-image |
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.
We should remove this.
| @@ -0,0 +1,2 @@ | |||
| <app-root ng-version="20.3.15" _nghost-ng-c772887568=""><div _ngcontent-ng-c772887568=""><div _ngcontent-ng-c772887568="" class="tabs"><button _ngcontent-ng-c772887568="" class="tab-button active"> Display Tests (Image & Video) </button><button _ngcontent-ng-c772887568="" class="tab-button"> Upload Test </button></div><div _ngcontent-ng-c772887568="" class="tab-content"><div _ngcontent-ng-c772887568="" class="display-tests"><h1 _ngcontent-ng-c772887568="">Image</h1><ik-image _ngcontent-ng-c772887568="" urlendpoint="https://ik.imagekit.io/demo/" src="default-image.jpg" alt="Image without ImageKit provider"><img ikbind="" src="https://ik.imagekit.io/demo/default-image.jpg?tr=w-640,c-at_max" alt="Image without ImageKit provider" srcset="https://ik.imagekit.io/demo/default-image.jpg?tr=w-384,c-at_max 1x, https://ik.imagekit.io/demo/default-image.jpg?tr=w-640,c-at_max 2x" loading="lazy" width="300" height="300"></ik-image><ik-image _ngcontent-ng-c772887568="" src="/default-image.jpg" alt="Image with ImageKit provider"><img ikbind="" src="https://ik.imagekit.io/demo/default-image.jpg?tr=w-640,c-at_max" alt="Image with ImageKit provider" srcset="https://ik.imagekit.io/demo/default-image.jpg?tr=w-384,c-at_max 1x, https://ik.imagekit.io/demo/default-image.jpg?tr=w-640,c-at_max 2x" loading="lazy" width="300" height="300" data-testid="hero-image" aria-describedby="image-description" data-track="analytics-id" crossorigin="anonymous" decoding="async"></ik-image><ik-image _ngcontent-ng-c772887568="" src="/default-image.jpg" alt="Image with width not number, should produce larger srcset" width="300px"><img ikbind="" src="https://ik.imagekit.io/demo/default-image.jpg?tr=w-3840,c-at_max" alt="Image with width not number, should produce larger srcset" srcset="https://ik.imagekit.io/demo/default-image.jpg?tr=w-640,c-at_max 640w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-750,c-at_max 750w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-828,c-at_max 828w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-1080,c-at_max 1080w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-1200,c-at_max 1200w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-1920,c-at_max 1920w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-2048,c-at_max 2048w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-3840,c-at_max 3840w" loading="lazy" width="300px"></ik-image><ik-image _ngcontent-ng-c772887568="" src="/default-image.jpg" alt="With transformation"><img ikbind="" src="https://ik.imagekit.io/demo/default-image.jpg?tr=h-100,w-100:w-640,c-at_max" alt="With transformation" srcset="https://ik.imagekit.io/demo/default-image.jpg?tr=h-100,w-100:w-384,c-at_max 1x, https://ik.imagekit.io/demo/default-image.jpg?tr=h-100,w-100:w-640,c-at_max 2x" loading="lazy" width="300" height="300"></ik-image><ik-image _ngcontent-ng-c772887568="" src="/default-image.jpg" alt="Image with queryParameters"><img ikbind="" src="https://ik.imagekit.io/demo/default-image.jpg?version=v1&tr=h-100,w-100:w-640,c-at_max" alt="Image with queryParameters" srcset="https://ik.imagekit.io/demo/default-image.jpg?version=v1&tr=h-100,w-100:w-384,c-at_max 1x, https://ik.imagekit.io/demo/default-image.jpg?version=v1&tr=h-100,w-100:w-640,c-at_max 2x" loading="lazy" width="300" height="300"></ik-image><ik-image _ngcontent-ng-c772887568="" src="/default-image.jpg" alt="Responsive image with sizes" sizes="(max-width: 600px) 100vw, 50vw"><img ikbind="" src="https://ik.imagekit.io/demo/default-image.jpg?tr=w-3840,c-at_max" alt="Responsive image with sizes" srcset="https://ik.imagekit.io/demo/default-image.jpg?tr=w-384,c-at_max 384w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-640,c-at_max 640w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-750,c-at_max 750w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-828,c-at_max 828w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-1080,c-at_max 1080w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-1200,c-at_max 1200w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-1920,c-at_max 1920w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-2048,c-at_max 2048w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-3840,c-at_max 3840w" sizes="(max-width: 600px) 100vw, 50vw" loading="lazy" width="300" height="300"></ik-image><ik-image _ngcontent-ng-c772887568="" src="/default-image.jpg" alt="Responsive image with sizes - should have very large srcset for all breakpoints" sizes="300px"><img ikbind="" src="https://ik.imagekit.io/demo/default-image.jpg?tr=w-3840,c-at_max" alt="Responsive image with sizes - should have very large srcset for all breakpoints" srcset="https://ik.imagekit.io/demo/default-image.jpg?tr=w-16,c-at_max 16w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-32,c-at_max 32w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-48,c-at_max 48w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-64,c-at_max 64w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-96,c-at_max 96w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-128,c-at_max 128w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-256,c-at_max 256w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-384,c-at_max 384w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-640,c-at_max 640w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-750,c-at_max 750w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-828,c-at_max 828w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-1080,c-at_max 1080w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-1200,c-at_max 1200w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-1920,c-at_max 1920w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-2048,c-at_max 2048w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-3840,c-at_max 3840w" sizes="300px" loading="lazy" width="300" height="300"></ik-image><ik-image _ngcontent-ng-c772887568="" urlendpoint="https://ik.imagekit.io/demo2/" src="/default-image.jpg" alt="Image with urlEndpoint override"><img ikbind="" src="https://ik.imagekit.io/demo2/default-image.jpg?tr=h-100,w-100:w-640,c-at_max" alt="Image with urlEndpoint override" srcset="https://ik.imagekit.io/demo2/default-image.jpg?tr=h-100,w-100:w-384,c-at_max 1x, https://ik.imagekit.io/demo2/default-image.jpg?tr=h-100,w-100:w-640,c-at_max 2x" loading="lazy" width="300" height="300"></ik-image><ik-image _ngcontent-ng-c772887568="" src="/default-image.jpg" alt="Image with className" classname="custom-class"><img ikbind="" src="https://ik.imagekit.io/demo/default-image.jpg?tr=w-640,c-at_max" alt="Image with className" srcset="https://ik.imagekit.io/demo/default-image.jpg?tr=w-384,c-at_max 1x, https://ik.imagekit.io/demo/default-image.jpg?tr=w-640,c-at_max 2x" loading="lazy" width="300" height="300" class="custom-class"></ik-image><ik-image _ngcontent-ng-c772887568="" src="/default-image.jpg" alt="Image with lazy loading eager" loading="eager"><img ikbind="" src="https://ik.imagekit.io/demo/default-image.jpg?tr=w-640,c-at_max" alt="Image with lazy loading eager" srcset="https://ik.imagekit.io/demo/default-image.jpg?tr=w-384,c-at_max 1x, https://ik.imagekit.io/demo/default-image.jpg?tr=w-640,c-at_max 2x" loading="eager" width="300" height="300"></ik-image><ik-image _ngcontent-ng-c772887568="" src="/default-image.jpg" alt="Image with path transformation" transformationposition="path"><img ikbind="" src="https://ik.imagekit.io/demo/tr:w-640,c-at_max/default-image.jpg" alt="Image with path transformation" srcset="https://ik.imagekit.io/demo/tr:w-384,c-at_max/default-image.jpg 1x, https://ik.imagekit.io/demo/tr:w-640,c-at_max/default-image.jpg 2x" loading="lazy" width="300" height="300"></ik-image><ik-image _ngcontent-ng-c772887568="" src="/default-image.jpg" alt="Image with path transformation + custom transformations" transformationposition="path"><img ikbind="" src="https://ik.imagekit.io/demo/tr:h-100,w-100:w-640,c-at_max/default-image.jpg" alt="Image with path transformation + custom transformations" srcset="https://ik.imagekit.io/demo/tr:h-100,w-100:w-384,c-at_max/default-image.jpg 1x, https://ik.imagekit.io/demo/tr:h-100,w-100:w-640,c-at_max/default-image.jpg 2x" loading="lazy" width="300" height="300"></ik-image><ik-image _ngcontent-ng-c772887568="" src="https://ik.imagekit.io/demo/default-image.jpg" alt="path not respected with absolute url" transformationposition="path"><img ikbind="" src="https://ik.imagekit.io/demo/default-image.jpg?tr=w-640,c-at_max" alt="path not respected with absolute url" srcset="https://ik.imagekit.io/demo/default-image.jpg?tr=w-384,c-at_max 1x, https://ik.imagekit.io/demo/default-image.jpg?tr=w-640,c-at_max 2x" loading="lazy" width="300" height="300"></ik-image><ik-image _ngcontent-ng-c772887568="" src="/default-image.jpg" alt="No width"><img ikbind="" src="https://ik.imagekit.io/demo/default-image.jpg?tr=w-3840,c-at_max" alt="No width" srcset="https://ik.imagekit.io/demo/default-image.jpg?tr=w-640,c-at_max 640w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-750,c-at_max 750w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-828,c-at_max 828w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-1080,c-at_max 1080w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-1200,c-at_max 1200w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-1920,c-at_max 1920w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-2048,c-at_max 2048w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-3840,c-at_max 3840w" loading="lazy"></ik-image><ik-image _ngcontent-ng-c772887568="" src="https://ik.imagekit.io/demo/default-image.jpg" alt="Custom deviceBreakpoints"><img ikbind="" src="https://ik.imagekit.io/demo/default-image.jpg?tr=w-800,c-at_max" alt="Custom deviceBreakpoints" srcset="https://ik.imagekit.io/demo/default-image.jpg?tr=w-200,c-at_max 200w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-400,c-at_max 400w, https://ik.imagekit.io/demo/default-image.jpg?tr=w-800,c-at_max 800w" loading="lazy"></ik-image><ik-image _ngcontent-ng-c772887568="" src="/default-image.jpg" alt="Image with responsive off" sizes="(max-width: 600px) 100vw, 50vw"><img ikbind="" src="https://ik.imagekit.io/demo/default-image.jpg" alt="Image with responsive off" sizes="(max-width: 600px) 100vw, 50vw" loading="lazy" width="300" height="300"></ik-image> | |||
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.
img is being wrapped inside ik-image which can be avoided.
…ideo in public API and component files for consistency
… app dependencies
Complete rewrite using standalone components
Exposes ik-image and ik-video component
Added a new test-app similar to that we have on react SDK
Dropped LQIP and Upload component
exposed buildSrc, buildTransformationString, upload, getResponsiveImageAttributes directly from @imagekit/javascript
added a passthrough attribute to pass properties directly to img/video elements (eg: passing data-id etc )