Skip to content

refactor: Break @calcom/app-store/components into individual component files#23637

Merged
hbjORbj merged 4 commits intomainfrom
chore/remove-duplicate-pages
Sep 6, 2025
Merged

refactor: Break @calcom/app-store/components into individual component files#23637
hbjORbj merged 4 commits intomainfrom
chore/remove-duplicate-pages

Conversation

@hbjORbj
Copy link
Copy Markdown
Contributor

@hbjORbj hbjORbj commented Sep 6, 2025

What does this PR do?

  • Part of Codebase Sanity Check in App Store Package
  • This PR gets rid of @calcom/app-store/components file and rather extracts the components into individual files

Result

Screenshot 2025-09-06 at 4 02 05 PM

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • N/A - I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Tests passing are sufficient

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Sep 6, 2025

Walkthrough

  • Removed packages/app-store/_components/AppConfiguration.tsx, including exports ConfigAppMap and AppConfiguration.
  • Pruned exports from packages/app-store/AppDependencyComponent.tsx, dropping AppConfiguration, InstallAppButton, and InstallAppButtonWithoutPlanCheck.
  • Added new components: packages/app-store/InstallAppButton.tsx and packages/app-store/InstallAppButtonWithoutPlanCheck.tsx, rehosting install-button logic and mutation handling.
  • Updated imports across web and packages to reference the new module paths (@calcom/app-store/InstallAppButton and @calcom/app-store/InstallAppButtonWithoutPlanCheck).
  • Adjusted internal import in _components/OmniInstallAppButton.tsx to the new InstallAppButton path.
  • No behavioral changes indicated beyond module relocations and API surface reshaping.

Possibly related PRs

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/remove-duplicate-pages

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@graphite-app graphite-app bot requested review from a team September 6, 2025 07:03
@keithwillcode keithwillcode added core area: core, team members only foundation labels Sep 6, 2025
@@ -1,19 +0,0 @@
import dynamic from "next/dynamic";
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed that this file isn't being consumed anywhere

@dosubot dosubot bot added the app-store area: app store, apps, calendar integrations, google calendar, outlook, lark, apple calendar label Sep 6, 2025
@vercel
Copy link
Copy Markdown

vercel bot commented Sep 6, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Sep 6, 2025 7:04am
cal-eu Ignored Ignored Sep 6, 2025 7:04am

@hbjORbj hbjORbj changed the title chore: remove duplicate pages refactor: Break @calcom/app-store/components into individual component files Sep 6, 2025
@hbjORbj hbjORbj enabled auto-merge (squash) September 6, 2025 07:07
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/features/apps/components/AppCard.tsx (1)

141-169: Enforce teams-plan gating in InstallAppButton
The teamsPlanRequired prop is never consumed—InstallAppButton just forwards it to InstallAppButtonWithoutPlanCheck, which ignores it. Add plan-checking logic in packages/app-store/InstallAppButton.tsx (or guard callers) to block installs when the user’s plan doesn’t meet teamsPlanRequired.

🧹 Nitpick comments (6)
apps/web/components/getting-started/components/AppConnectionItem.tsx (1)

111-115: Harden cookie write (encode value + Secure when HTTPS).

Prevents issues with special chars and enforces Secure on production.

-                document.cookie = `return-to=${window.location.href};path=/;max-age=3600;SameSite=Lax`;
+                const secure = window.location.protocol === "https:" ? ";Secure" : "";
+                document.cookie = `return-to=${encodeURIComponent(
+                  window.location.href
+                )};path=/;max-age=3600;SameSite=Lax${secure}`;
packages/features/apps/components/AppList.tsx (1)

67-70: Localize toast message.

Follow repo i18n guideline for user-facing text.

-    showToast("Default app updated successfully", "success");
+    showToast(t("default_app_updated_successfully"), "success");
packages/features/calendars/AdditionalCalendarSelector.tsx (1)

29-34: Localize “Add new calendars” option label.

Avoid hardcoded strings in UI.

-  options.push({
-    label: "Add new calendars",
+  options.push({
+    label: t("add_new_calendars"),
packages/features/apps/components/AppCard.tsx (2)

94-95: Localize alt text.

Per TSX localization guideline, avoid raw strings; translate “Logo”.

-          alt={`${app.name} Logo`}
+          alt={`${app.name} ${t("logo")}`}

190-191: Localize “Template” badge text.

Use t() instead of a raw string.

-          <span className="bg-error rounded-md px-2 py-1 text-sm font-normal text-red-800">Template</span>
+          <span className="bg-error rounded-md px-2 py-1 text-sm font-normal text-red-800">{t("template")}</span>
packages/app-store/InstallAppButton.tsx (1)

16-20: Avoid unnecessary wrapper div when no class provided.

Reduce DOM depth by returning the child directly if wrapperClassName is falsy.

-  return (
-    <div className={props.wrapperClassName}>
-      <InstallAppButtonWithoutPlanCheck {...props} />
-    </div>
-  );
+  return props.wrapperClassName ? (
+    <div className={props.wrapperClassName}>
+      <InstallAppButtonWithoutPlanCheck {...props} />
+    </div>
+  ) : (
+    <InstallAppButtonWithoutPlanCheck {...props} />
+  );
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 53bf5a7 and e570aa9.

📒 Files selected for processing (10)
  • apps/web/components/apps/AppPage.tsx (1 hunks)
  • apps/web/components/apps/CalendarListContainer.tsx (1 hunks)
  • apps/web/components/getting-started/components/AppConnectionItem.tsx (1 hunks)
  • packages/app-store/AppDependencyComponent.tsx (0 hunks)
  • packages/app-store/InstallAppButton.tsx (1 hunks)
  • packages/app-store/InstallAppButtonWithoutPlanCheck.tsx (1 hunks)
  • packages/app-store/_components/OmniInstallAppButton.tsx (1 hunks)
  • packages/features/apps/components/AppCard.tsx (1 hunks)
  • packages/features/apps/components/AppList.tsx (1 hunks)
  • packages/features/calendars/AdditionalCalendarSelector.tsx (1 hunks)
💤 Files with no reviewable changes (1)
  • packages/app-store/AppDependencyComponent.tsx
✅ Files skipped from review due to trivial changes (2)
  • apps/web/components/apps/CalendarListContainer.tsx
  • apps/web/components/apps/AppPage.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Always use t() for text localization in frontend code; direct text embedding should trigger a warning

Files:

  • packages/features/calendars/AdditionalCalendarSelector.tsx
  • packages/app-store/InstallAppButton.tsx
  • packages/features/apps/components/AppList.tsx
  • packages/app-store/_components/OmniInstallAppButton.tsx
  • packages/app-store/InstallAppButtonWithoutPlanCheck.tsx
  • packages/features/apps/components/AppCard.tsx
  • apps/web/components/getting-started/components/AppConnectionItem.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • packages/features/calendars/AdditionalCalendarSelector.tsx
  • packages/app-store/InstallAppButton.tsx
  • packages/features/apps/components/AppList.tsx
  • packages/app-store/_components/OmniInstallAppButton.tsx
  • packages/app-store/InstallAppButtonWithoutPlanCheck.tsx
  • packages/features/apps/components/AppCard.tsx
  • apps/web/components/getting-started/components/AppConnectionItem.tsx
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • packages/features/calendars/AdditionalCalendarSelector.tsx
  • packages/app-store/InstallAppButton.tsx
  • packages/features/apps/components/AppList.tsx
  • packages/app-store/_components/OmniInstallAppButton.tsx
  • packages/app-store/InstallAppButtonWithoutPlanCheck.tsx
  • packages/features/apps/components/AppCard.tsx
  • apps/web/components/getting-started/components/AppConnectionItem.tsx
🧬 Code graph analysis (2)
packages/app-store/InstallAppButton.tsx (2)
packages/app-store/types.d.ts (1)
  • InstallAppButtonProps (37-47)
packages/app-store/InstallAppButtonWithoutPlanCheck.tsx (1)
  • InstallAppButtonWithoutPlanCheck (11-41)
packages/app-store/InstallAppButtonWithoutPlanCheck.tsx (2)
packages/app-store/types.d.ts (1)
  • InstallAppButtonProps (37-47)
packages/app-store/apps.browser.generated.tsx (1)
  • InstallAppButtonMap (7-12)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (5)
apps/web/components/getting-started/components/AppConnectionItem.tsx (1)

4-4: Import path update looks good.

packages/features/apps/components/AppList.tsx (1)

3-3: Import path consolidation to @calcom/app-store/InstallAppButton is correct.

packages/app-store/_components/OmniInstallAppButton.tsx (1)

8-8: Switched to central InstallAppButton import — LGTM.

packages/features/calendars/AdditionalCalendarSelector.tsx (1)

3-3: Import path migration looks correct.

packages/features/apps/components/AppCard.tsx (1)

6-6: No lingering old imports detected; changes approved
Ripgrep search returned no matches for @calcom/app-store/components. All imports updated.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Sep 6, 2025

E2E results are ready!

@hbjORbj hbjORbj merged commit 7a1c780 into main Sep 6, 2025
68 of 70 checks passed
@hbjORbj hbjORbj deleted the chore/remove-duplicate-pages branch September 6, 2025 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app-store area: app store, apps, calendar integrations, google calendar, outlook, lark, apple calendar core area: core, team members only foundation ready-for-e2e size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants