Skip to content

Conversation

@benlife5
Copy link
Contributor

@benlife5 benlife5 commented Nov 6, 2025

The migration props need to be frozen with the current prop shape. If they remain typed, future updates to the prop shape will fail the build.

If we need to modify the migrations as part of bug bash fixes, we can temporarily add the types back for the affected migration

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

Walkthrough

This PR removes TypeScript type satisfaction assertions (satisfies WithId<...>) from 13 migration files within the visual-editor package's components directory. Each file contains slot definitions for various sections (event, promo, product, insight, teams, FAQ, testimonials, photo gallery, reviews, directory, and headers). The changes eliminate compile-time type constraints by removing the satisfies wrappers and their associated imports, while preserving all runtime logic and data transformation behavior unchanged.

Possibly related PRs

  • fix: slots qa part 2 #859: Modifies the same migration file (0036_testimonials_section_slots.ts) with related timestamp/slot typing changes

Suggested labels

create-dev-release

Suggested reviewers

  • mkilpatrick
  • colton-demetriou
  • asanehisa

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'chore: remove types from migrations' accurately describes the main change: removing TypeScript type assertions from multiple migration files to freeze prop shapes.
Description check ✅ Passed The description explains the rationale for removing types from migrations: to freeze prop shapes and prevent future prop-shape updates from breaking builds, with guidance on temporarily re-adding types if needed.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch remove-types

📜 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 c92f481 and 0fc34c2.

📒 Files selected for processing (13)
  • packages/visual-editor/src/components/migrations/0025_event_section_slots.ts (8 hunks)
  • packages/visual-editor/src/components/migrations/0028_organize_cta_wrapper_props.ts (2 hunks)
  • packages/visual-editor/src/components/migrations/0029_promo_section_slots.ts (5 hunks)
  • packages/visual-editor/src/components/migrations/0031_product_section_slots.ts (8 hunks)
  • packages/visual-editor/src/components/migrations/0032_insight_section_slots.ts (9 hunks)
  • packages/visual-editor/src/components/migrations/0033_teams_section_slots.ts (7 hunks)
  • packages/visual-editor/src/components/migrations/0034_slotify_faq_section.ts (4 hunks)
  • packages/visual-editor/src/components/migrations/0035_nearby_location_slots.ts (2 hunks)
  • packages/visual-editor/src/components/migrations/0036_testimonials_section_slots.ts (6 hunks)
  • packages/visual-editor/src/components/migrations/0037_photo_gallery_slots.ts (1 hunks)
  • packages/visual-editor/src/components/migrations/0038_reviews_section_slots.ts (1 hunks)
  • packages/visual-editor/src/components/migrations/0039_slotify_directory.ts (7 hunks)
  • packages/visual-editor/src/components/migrations/0040_expanded_header_slots.ts (5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: benlife5
Repo: yext/visual-editor PR: 862
File: packages/visual-editor/src/utils/schema/resolveSchema.ts:118-135
Timestamp: 2025-11-06T14:55:12.385Z
Learning: In `packages/visual-editor/src/utils/schema/resolveSchema.ts`, the `OpeningHoursSchema` and `PhotoGallerySchema` functions from `yext/pages-components` contain internal type validation and handle invalid inputs gracefully (returning empty objects or undefined) rather than throwing TypeErrors, so no pre-validation guards are needed before calling them.
📚 Learning: 2025-11-06T14:55:12.385Z
Learnt from: benlife5
Repo: yext/visual-editor PR: 862
File: packages/visual-editor/src/utils/schema/resolveSchema.ts:118-135
Timestamp: 2025-11-06T14:55:12.385Z
Learning: In `packages/visual-editor/src/utils/schema/resolveSchema.ts`, the `OpeningHoursSchema` and `PhotoGallerySchema` functions from `yext/pages-components` contain internal type validation and handle invalid inputs gracefully (returning empty objects or undefined) rather than throwing TypeErrors, so no pre-validation guards are needed before calling them.

Applied to files:

  • packages/visual-editor/src/components/migrations/0037_photo_gallery_slots.ts
  • packages/visual-editor/src/components/migrations/0038_reviews_section_slots.ts
  • packages/visual-editor/src/components/migrations/0029_promo_section_slots.ts
  • packages/visual-editor/src/components/migrations/0040_expanded_header_slots.ts
  • packages/visual-editor/src/components/migrations/0036_testimonials_section_slots.ts
  • packages/visual-editor/src/components/migrations/0025_event_section_slots.ts
  • packages/visual-editor/src/components/migrations/0039_slotify_directory.ts
🔇 Additional comments (13)
packages/visual-editor/src/components/migrations/0037_photo_gallery_slots.ts (1)

38-38: LGTM - Type assertion successfully removed.

The removal of the satisfies WithId<HeadingTextProps> assertion aligns with the PR objective to freeze migration prop shapes and prevent future type changes from breaking builds.

packages/visual-editor/src/components/migrations/0035_nearby_location_slots.ts (1)

29-29: LGTM - Type assertions successfully removed.

All three type assertion removals (HeadingTextSlot, NearbyLocationCardsWrapper, and outer NearbyLocationsSection) are consistent with the PR objective.

Also applies to: 52-52, 56-56

packages/visual-editor/src/components/migrations/0028_organize_cta_wrapper_props.ts (1)

19-19: LGTM - Type assertions successfully removed.

Both CTAWrapper and CTAGroup transformations now return plain objects without type constraints, aligning with the migration freeze strategy.

Also applies to: 29-29

packages/visual-editor/src/components/migrations/0029_promo_section_slots.ts (1)

74-74: LGTM - Type assertions successfully removed across all slots.

All five slot definitions (HeadingSlot, DescriptionSlot, VideoSlot, ImageSlot, CTASlot) have been updated consistently, maintaining runtime behavior while removing compile-time type constraints.

Also applies to: 99-99, 113-113, 154-154, 189-189

packages/visual-editor/src/components/migrations/0038_reviews_section_slots.ts (1)

62-62: LGTM - Type assertions successfully removed.

Both the inner HeadingTextSlot and outer ReviewsSection type assertions have been cleanly removed while preserving the transformation logic.

Also applies to: 66-66

packages/visual-editor/src/components/migrations/0040_expanded_header_slots.ts (1)

35-35: LGTM - Type assertions successfully removed across nested header slots.

All seven type assertion removals throughout the expanded header slot structure are consistent with the PR objective, covering both primary and secondary header components.

Also applies to: 39-39, 82-82, 105-105, 134-134, 148-148, 152-152

packages/visual-editor/src/components/migrations/0033_teams_section_slots.ts (1)

69-69: LGTM - Type assertions successfully removed across team section slots.

All eight type assertion removals are consistent with the PR objective, covering both individual card slots (ImageSlot, NameSlot, TitleSlot, CTASlot) and wrapper-level components while preserving the complete transformation logic.

Also applies to: 94-94, 116-116, 199-199, 209-209, 235-235, 251-251, 255-255

packages/visual-editor/src/components/migrations/0034_slotify_faq_section.ts (1)

58-58: LGTM - Type assertions successfully removed across FAQ slots.

All four type assertion removals (QuestionSlot, AnswerSlot, Card, FAQsWrapperSlot) are consistent with the PR objective, maintaining the FAQ transformation logic while removing compile-time type constraints.

Also applies to: 80-80, 90-90, 129-129

packages/visual-editor/src/components/migrations/0025_event_section_slots.ts (1)

82-266: Runtime card construction remains intact

Confirmed the slot payloads retain their original shape; only the compile-time satisfies guards were removed, which aligns with the stated goal of freezing migration prop shapes. No functional concerns spotted.

packages/visual-editor/src/components/migrations/0032_insight_section_slots.ts (1)

66-255: Section slot payload stays consistent

The removal of satisfies WithId<…> didn’t alter the emitted slot data; card IDs, parentData, and conditional wiring still mirror the prior behavior. Looks good.

packages/visual-editor/src/components/migrations/0036_testimonials_section_slots.ts (1)

39-167: No behavioral delta after stripping satisfies

Testimonial card generation still supplies the same slot props and parent bindings; only the compile-time guards are gone. Change meets the PR objective without side effects.

packages/visual-editor/src/components/migrations/0031_product_section_slots.ts (1)

70-236: Product slot definitions preserved

All slot structures, IDs, and parentData wiring remain identical; dropping the satisfies assertions doesn’t impact runtime behavior. 👍

packages/visual-editor/src/components/migrations/0039_slotify_directory.ts (1)

47-188: Directory migration continues to emit the same structure

Verified that the directory card slots and wrapper return value are untouched aside from the removed satisfies clauses, so behavior stays the same while meeting the goal of decoupling from future prop type changes.


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

@benlife5 benlife5 merged commit 0499278 into fall-2025-slot-ify-components Nov 10, 2025
14 checks passed
@benlife5 benlife5 deleted the remove-types branch November 10, 2025 14:58
@coderabbitai coderabbitai bot mentioned this pull request Nov 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants