Skip to content

Comments

chore: add disable image extension functionality#6706

Merged
sriramveeraghanta merged 2 commits intopreviewfrom
chore/image-extension
Mar 6, 2025
Merged

chore: add disable image extension functionality#6706
sriramveeraghanta merged 2 commits intopreviewfrom
chore/image-extension

Conversation

@aaryan610
Copy link
Member

@aaryan610 aaryan610 commented Mar 5, 2025

Description

This PR adds the functionality to disable the editor image extension.

Type of Change

  • Improvement (change that would cause existing functionality to not work as expected)

Summary by CodeRabbit

  • New Features
    • Introduced dynamic image functionalities throughout the editor, including conditional display in both editing and read-only modes, as well as an image option in the slash-command menu.
  • Bug Fixes
    • Improved error handling for image uploads and status tracking, leading to a more stable and reliable image experience.
  • Chores
    • Updated experimental configurations to optimize package imports for enhanced performance.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 5, 2025

Walkthrough

This pull request improves the robustness of various image-related functionalities by introducing optional chaining to safely access properties when the image component may be undefined. It adjusts the control flow in extension handling and slash command filtering, updates type definitions and interface method signatures, and adds new properties for tracking upload status. Additionally, an experimental Next.js configuration is introduced for optimizing package imports during build.

Changes

File(s) Change Summary
packages/editor/src/core/extensions/custom-image/components/image-node.tsx, packages/editor/src/core/extensions/custom-image/components/upload-status.tsx, packages/editor/src/core/hooks/use-editor.ts, packages/editor/src/core/hooks/use-file-upload.ts Implemented optional chaining for properties such as maxFileSize, assetsUploadStatus, and uploadInProgress to prevent runtime errors when imageComponent is undefined.
packages/editor/src/core/extensions/extensions.tsx, packages/editor/src/core/extensions/read-only-extensions.tsx, packages/editor/src/core/extensions/slash-commands/command-items-list.tsx Restructured control flow to conditionally include image-related extensions and dynamically merge slash command options based on disabledExtensions.
packages/editor/src/core/extensions/custom-image/custom-image.ts Updated the updateAssetsUploadStatus method signature to be optional, allowing greater flexibility in the imageComponent interface.
packages/editor/src/core/extensions/image/image-component-without-props.tsx Added new assetsUploadStatus property within the addStorage method to track asset upload statuses.
packages/editor/src/core/types/extensions.ts Expanded the TExtensions type by adding a new literal "image".
web/next.config.js Introduced a new experimental configuration (optimizePackageImports) to optimize package imports during the build process.

Sequence Diagram(s)

sequenceDiagram
    participant Editor
    participant ExtensionLoader as "Extension Loader"
    Editor->>ExtensionLoader: Request Initialization
    ExtensionLoader->>ExtensionLoader: Check if "image" is disabled
    alt Image Extension Enabled
        ExtensionLoader->>ExtensionLoader: Add ImageExtension & CustomImageExtension
    else
        ExtensionLoader->>ExtensionLoader: Skip image extensions
    end
    ExtensionLoader->>Editor: Return Extensions List
Loading
sequenceDiagram
    participant Editor
    participant SlashCommandHandler as "Slash Command Handler"
    Editor->>SlashCommandHandler: Request filtered command sections
    SlashCommandHandler->>SlashCommandHandler: Build internalAdditionalOptions (conditionally for "image")
    SlashCommandHandler->>SlashCommandHandler: Merge external options with internal ones
    SlashCommandHandler->>Editor: Return ordered command sections
Loading

Possibly related PRs

Suggested labels

🧹chore, ✨feature

Suggested reviewers

  • SatishGandham
  • Palanikannan1437

Poem

I'm a hopping rabbit in the codeyard bright,
Skipping past pitfalls with safe checks in sight.
Optional chaining helps my uploads flow,
No runtime errors—just carrots in tow!
With conditional extensions and commands that gleam,
My code garden blooms like a dream.
Hooray for robust code, now that's the theme!


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f01d82a and 6923225.

📒 Files selected for processing (11)
  • packages/editor/src/core/extensions/custom-image/components/image-node.tsx (1 hunks)
  • packages/editor/src/core/extensions/custom-image/components/upload-status.tsx (1 hunks)
  • packages/editor/src/core/extensions/custom-image/custom-image.ts (1 hunks)
  • packages/editor/src/core/extensions/extensions.tsx (3 hunks)
  • packages/editor/src/core/extensions/image/image-component-without-props.tsx (1 hunks)
  • packages/editor/src/core/extensions/read-only-extensions.tsx (2 hunks)
  • packages/editor/src/core/extensions/slash-commands/command-items-list.tsx (3 hunks)
  • packages/editor/src/core/hooks/use-editor.ts (3 hunks)
  • packages/editor/src/core/hooks/use-file-upload.ts (1 hunks)
  • packages/editor/src/core/types/extensions.ts (1 hunks)
  • web/next.config.js (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Analyze (python)
🔇 Additional comments (22)
packages/editor/src/core/extensions/image/image-component-without-props.tsx (1)

51-51: Adding assetsUploadStatus property to support tracking image upload status.

This addition aligns with other image extension improvements in the PR, providing a dedicated space to track the upload status of assets. The initialization as an empty object is appropriate for this use case.

packages/editor/src/core/extensions/custom-image/components/image-node.tsx (1)

79-79: Good use of optional chaining for safe property access.

Adding the optional chaining operator (?.) here prevents potential runtime errors when the image component is disabled or undefined. This improves the code's robustness and aligns with the PR's objective of making the image extension functionality optional.

packages/editor/src/core/hooks/use-file-upload.ts (1)

24-26: Good defensive programming with null check.

Adding this conditional check prevents potential runtime errors when attempting to access properties on imageComponent when it might be undefined. This is consistent with the other changes in this PR that make the code more robust when the image extension is disabled.

packages/editor/src/core/extensions/custom-image/custom-image.ts (1)

25-25:

Details

❓ Verification inconclusive

Making updateAssetsUploadStatus optional improves extension flexibility.

Adding the optional modifier (?) to this method signature is appropriate for supporting scenarios where the image extension might be disabled. This change ensures that the TypeScript interface correctly reflects that this method may not always be available.

However, ensure that any calls to this method throughout the codebase use optional chaining (?.) to prevent runtime errors.


🏁 Script executed:

#!/bin/bash
# Search for direct calls to updateAssetsUploadStatus without optional chaining
rg "updateAssetsUploadStatus\(" --type ts --type tsx

Length of output: 82


Below is an updated verification shell script that targets both “.ts” and “.tsx” files without relying on ripgrep’s built‐in file types:

Please run the above script to manually verify that all calls to updateAssetsUploadStatus use optional chaining (i.e. ?.). If the script produces no output, it suggests that no direct (non-optional) calls exist and that the optional signature change is safe. If any output appears, you’ll need to update those call sites accordingly.

Action required:
• Run the provided script and confirm that there are no non-optional calls to updateAssetsUploadStatus.


🏁 Script executed:

#!/bin/bash
# Search for usages of updateAssetsUploadStatus without optional chaining in .ts and .tsx files.
# This uses a negative lookbehind to catch calls using a plain dot instead of an optional chaining operator.
rg -P "(?<!\?)\.updateAssetsUploadStatus\(" -g "*.ts" -g "*.tsx"

Length of output: 188


Verification Update: Confirm Optional Chaining for updateAssetsUploadStatus

The change to mark updateAssetsUploadStatus as optional in
packages/editor/src/core/extensions/custom-image/custom-image.ts (line 25) is appropriate for flexibility when the image extension is disabled.

Please ensure that all calls to this method use optional chaining (?.) to avoid runtime errors. To verify this manually, run the following command (without the `` marker):

If the command produces no output, it confirms that there are no direct (non-optional) calls to updateAssetsUploadStatus. Otherwise, please update those call sites to use ?..


🏁 Script executed:

rg -P "(?<!\?)\.updateAssetsUploadStatus\(" -g "*.ts" -g "*.tsx"

Length of output: 66


Verify that Optional Chaining is Consistently Applied

The change in
packages/editor/src/core/extensions/custom-image/custom-image.ts (line 25) to mark updateAssetsUploadStatus as optional is valid and makes the extension more flexible when it’s disabled. The updated signature

updateAssetsUploadStatus?: (updatedStatus: TFileHandler["assetsUploadStatus"]) => () => void;

accurately indicates that the method may not be available, which is helpful in managing conditional usage scenarios.

Please ensure that all invocations of updateAssetsUploadStatus across the codebase are using optional chaining (i.e. ?.updateAssetsUploadStatus(…)). A command such as

rg -P "(?<!\?)\.updateAssetsUploadStatus\(" -g "*.ts" -g "*.tsx"

is expected to return no matches. Although the above command didn’t produce output, a manual verification is recommended to confidently confirm that no direct (non-optional) calls exist. If any are found, those call sites need to be updated accordingly.

packages/editor/src/core/extensions/custom-image/components/upload-status.tsx (1)

19-19: Added optional chaining for safer property access.

This change introduces optional chaining (?.) to safely access the assetsUploadStatus property when imageComponent might be undefined. This is a good defensive programming practice that prevents potential runtime errors.

web/next.config.js (1)

78-140:

Details

❓ Verification inconclusive

Added Next.js experimental package import optimization.

This configuration leverages Next.js' experimental feature to optimize package imports, which can improve application performance and bundle size by making imports more efficient. The comprehensive list covers various components, libraries, and services across the application.

Worth noting:

  • This is an experimental Next.js feature, so ensure it works as expected across environments
  • The extensive list suggests a thorough analysis of imports to optimize

🌐 Web query:

What does the Next.js optimizePackageImports experimental feature do?

Error during web search


Experimental Optimize Package Imports Configuration Verified

The usage of Next.js’s experimental optimizePackageImports in web/next.config.js is correctly set up to target a broad set of components, libraries, and services. The comprehensive list should help reduce bundle sizes and improve import efficiency. However, since this feature remains experimental, please ensure that it’s thoroughly tested across all target environments.

  • Configuration correctly leverages Next.js’s experimental optimization for package imports.
  • The exhaustive list indicates a careful review of import paths.
  • Remains experimental: verify behavior in each environment.
packages/editor/src/core/types/extensions.ts (1)

1-1: Added 'image' to TExtensions type.

This is a good enhancement that aligns with the PR objective of allowing users to disable the editor image extension. By adding "image" to the TExtensions type, you're ensuring proper type safety when disabling this extension.

packages/editor/src/core/extensions/extensions.tsx (4)

53-54: Restructured extensions initialization for better modularity.

Converting the direct return to an explicit array initialization improves code organization and prepares for the conditional inclusion of image extensions.


148-148: Added optional chaining for safer property access.

This change introduces optional chaining (?.) to safely access the uploadInProgress property when imageComponent might be undefined, preventing potential runtime errors when the image extension is disabled.


187-188: Fixed TypeScript error with proper error suppression.

The TypeScript error suppression is appropriately applied with a clear comment explaining why it's needed. This is a good practice when dealing with third-party library type issues.


176-185:

Details

✅ Verification successful

Implemented conditional image extension inclusion.

This is the key implementation of the PR objective - enabling users to disable the image extension. The code now properly checks the disabledExtensions array and only adds the image-related extensions when they're not disabled.

Let's check if there are any other places in the codebase where image extension usage might need similar conditional checks:


🏁 Script executed:

#!/bin/bash
# Search for other direct references to image extensions that might need conditional checks
rg -A 3 -B 3 "ImageExtension|CustomImageExtension" --no-filename

Length of output: 4758


Conditional Image Extension Inclusion Verified

I've audited the codebase and confirmed that the image extension is conditionally added in both key places—once for the normal image extension and once for the read-only variant—using if (!disabledExtensions.includes("image")). No unguarded references to ImageExtension, CustomImageExtension, ReadOnlyImageExtension, or CustomReadOnlyImageExtension were found that might require a similar conditional check. The definitions for other image extension variants (like ImageExtensionWithoutProps) appear to be only used for extension creation and do not affect runtime inclusion.

  • packages/editor/src/core/extensions/extensions.tsx:
    • Both the standard and read-only image extension blocks are properly guarded by the disabledExtensions check.
  • No additional instances in the codebase require this conditional handling.

The implementation is consistent with the PR objective of allowing users to disable the image extension.

packages/editor/src/core/extensions/read-only-extensions.tsx (3)

44-131: Good structural refactoring for conditionally loading extensions.

The creation of an extensions array that collects all non-image extensions is a clean approach that improves code organization.


133-142: Well-implemented conditional image extension loading.

This implementation properly aligns with the PR objective of allowing users to disable the image extension functionality. The code checks if "image" is not included in the disabledExtensions array before adding the image-related extensions.


144-145: LGTM: Return statement is correctly updated.

The return statement now uses the extensions array that was constructed with conditional image extension support.

packages/editor/src/core/extensions/slash-commands/command-items-list.tsx (4)

46-46: LGTM: Proper type import for additional slash command options.

The import statement correctly adds TSlashCommandAdditionalOption type, which is necessary for the refactored code.


57-57: Good parameter renaming for clarity.

Renaming additionalOptions to externalAdditionalOptions improves code clarity by distinguishing between internal and external option sources.


278-291: Well-structured conditional image command implementation.

The implementation correctly creates an internal array of additional slash command options and conditionally adds the image command based on the disabledExtensions setting, which aligns with the PR's objective.


293-296: Appropriate ordering of command options.

The order ensures that internal options have priority, followed by external options and core editor options, which maintains a consistent user experience.

packages/editor/src/core/hooks/use-editor.ts (4)

114-114: Added defensive programming with optional chaining.

This change correctly adds optional chaining to safely handle cases where imageComponent might be undefined when the image extension is disabled, preventing potential runtime errors.


132-132: Safely accessing potentially unavailable commands.

The addition of optional chaining for updateAssetsUploadStatus ensures that the code won't throw an error if this command is unavailable when the image extension is disabled.


224-224: Consistent optional chaining for component access.

This change maintains consistency with the other updates in this file by adding optional chaining when accessing imageComponent in the isEditorReadyToDiscard method.


1-296:

Details

❓ Verification inconclusive

Verify completeness of null safety implementation.

While the current changes add optional chaining in key places, it would be good to verify that all access points to imageComponent have been made null-safe throughout the codebase.


🏁 Script executed:

#!/bin/bash
# Description: Find potential places where imageComponent might be accessed without optional chaining
# This could identify other places that need similar updates

# Look for direct access to imageComponent that might not be null-safe
rg "storage\.imageComponent\." --type ts --type tsx

# Compare with pattern that includes optional chaining
rg "storage\.imageComponent\?\." --type ts --type tsx

Length of output: 165


Additional Verification Required: Confirm Null Safety for imageComponent Across the Codebase

The optional chaining in the file packages/editor/src/core/hooks/use-editor.ts (e.g. in the checks for uploadInProgress) appears correct. However, the initial search for potential unsafe accesses was inconclusive due to errors with file type recognition. To improve confidence, please run the following updated shell script to search for all occurrences of storage.imageComponent in both .ts and .tsx files:

#!/bin/bash
echo "Unsafe occurrences (without optional chaining):"
rg 'storage\.imageComponent\.' -g "*.ts" -g "*.tsx"

echo "Safe occurrences (with optional chaining):"
rg 'storage\.imageComponent\?\.' -g "*.ts" -g "*.tsx"

After reviewing the output, verify that all access points use optional chaining where necessary. If any unsafe occurrences are found, further updates may be required.

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@sriramveeraghanta sriramveeraghanta merged commit f65b9a4 into preview Mar 6, 2025
5 of 6 checks passed
@sriramveeraghanta sriramveeraghanta deleted the chore/image-extension branch March 6, 2025 10:33
@sriramveeraghanta sriramveeraghanta added this to the v0.25.2 milestone Mar 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants