feat: enhance upload configuration with additional Uppy and XHRUpload… #553
Conversation
… options - Add support for passing custom Uppy and XHRUpload configuration options - Update `IUploadConfig` interface to include `uppyConfig` and `xhrConfig` - Spread additional configuration options in `createUploader`
WalkthroughThis update enhances the file upload configuration in the editor and core packages by introducing additional configurable options for Uppy and XHRUpload. The modifications include adding spread operator lines in the uploader creation function and extending the upload configuration interface with new optional properties. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Uploader
participant Uppy
participant XHRUpload
Client->>Uploader: Initialize upload with config (including uppyConfig, xhrConfig)
Uploader->>Uppy: Create instance with base config + ...uppyConfig
Uploader->>XHRUpload: Create instance with base config + ...xhrConfig
Uppy-->>Uploader: Return Uppy instance
XHRUpload-->>Uploader: Return XHRUpload instance
Uploader-->>Client: Return configured uploader
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/core/src/upload/interface.ts (1)
44-47: Consider adding type safety for advanced usersWhile
Record<string, any>provides maximum flexibility, consider adding more specific type definitions or references to Uppy types for better developer experience and type safety. This could be done by importing and using Uppy's own type definitions if available.// Example of how this could be implemented: import { UppyOptions } from '@uppy/core'; import { XHRUploadOptions } from '@uppy/xhr-upload'; // Then in the interface: uppyConfig?: Partial<UppyOptions>; xhrConfig?: Partial<XHRUploadOptions>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/twelve-seahorses-cross.md(1 hunks)packages/core/src/upload/createUploader.ts(1 hunks)packages/core/src/upload/interface.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test-e2e
- GitHub Check: test
🔇 Additional comments (6)
.changeset/twelve-seahorses-cross.md (1)
1-7: LGTM: Changeset correctly documents the changeThe changeset properly documents the feature enhancement to both affected packages with appropriate version updates (patch) and clear description.
packages/core/src/upload/createUploader.ts (3)
59-59: Good enhancement for flexible configurationThe spread operator allows for passing additional Uppy configuration options, providing users with more flexibility without modifying the core implementation.
68-68: Good enhancement for XHRUpload configurationAdding the spread operator for XHRUpload options allows users to configure advanced XHRUpload settings without changing the core implementation.
61-63:❓ Verification inconclusive
Verify potential override concerns
The position of the spread operator after core configuration values is correct, ensuring that user-provided configs can override default values if needed. However, be aware that this could potentially allow users to override critical configurations like
endpointandheaderswhich might lead to unexpected behavior.
🏁 Script executed:
#!/bin/bash # Check if there are any usage examples or documentation that mention uppyConfig or xhrConfig rg -A 5 -B 5 "uppyConfig|xhrConfig" --type mdLength of output: 47
🏁 Script executed:
#!/bin/bash # Check how the spread operator is applied in createUploader.ts to merge user configs rg "\.\.\." packages/core/src/upload/createUploader.tsLength of output: 150
Configuration Merge Override Check
The implementation in
packages/core/src/upload/createUploader.tscorrectly positions the spread operators after setting the core configuration values:endpoint: url, headers, formData: true,This ensures that any values provided via
config.uppyConfigandconfig.xhrConfig(as seen in the spread for merging additional options) will override the defaults. Please verify that allowing user-provided overrides for critical configurations likeendpointandheadersis intentional and won’t lead to unexpected behavior.packages/core/src/upload/interface.ts (2)
44-45: LGTM: Well-documented interface additionThe addition of
uppyConfigwith appropriate documentation comment is clear and follows the existing style of the interface. UsingRecord<string, any>is appropriate for this case as Uppy accepts various configuration options.
46-47: LGTM: Well-documented interface additionThe addition of
xhrConfigwith appropriate documentation comment is clear and follows the existing style of the interface. UsingRecord<string, any>is appropriate for this case as XHRUpload accepts various configuration options.
… options
IUploadConfiginterface to includeuppyConfigandxhrConfigcreateUploaderChanges Overview
Implementation Approach
Testing Done
Verification Steps
Additional Notes
Checklist
Related Issues
Summary by CodeRabbit