Skip to content
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

fix: resolve type error by inlining DocumentComponents definition #6703

Merged
merged 2 commits into from
May 26, 2024

Conversation

ricokahler
Copy link
Contributor

@ricokahler ricokahler commented May 16, 2024

Description

This PR addresses a type error caused by the missing 'preview' property in 'DocumentComponents'. The error, "Object literal may only specify known properties, and 'preview' does not exist in type 'DocumentComponents'", was resolved by inlining the type definitions within the module augmentation. This ensures TypeScript recognizes the 'preview' property as part of 'DocumentComponents' and avoids naming collisions that were causing the build process to prefer the incorrect type.

Note that this should not change the public API of the package at all due to the re-export but I may be missing something. An alternative would be to rename the DocumentComponents interface to something that doesn't collide.

Closes #6542

The fix involves:

  • Moving the component type definitions from './definitionExtensions' into the '@sanity/types' module.
  • Inlining type definitions to ensure the build process creates the correct .d.ts files.
  • Using a type import to prevent any impact on runtime.

This change was necessary to maintain consistency across the codebase and to resolve the type error encountered during the build process.

What to review

  • Verify that the type definitions for DocumentComponents are correctly inlined within the module augmentation.
  • Check that the type error related to the 'preview' property is resolved.
  • Ensure there are no cyclical dependencies introduced by this change.
  • Review the build process to confirm that the .d.ts files are correctly generated.

Added @stipsan as a reviewer to verify this has no affect on the build and to give insight on how the build outputs declaration files.

Testing

I manually tested the result by running the build and copying the resulting .d.ts files into a fresh project that previously showed the issue. The type error was gone, confirming that the fix works as intended.

Notes for release

Resolves a type error caused by the missing 'preview' property in 'DocumentComponents'.

- Addressed type error with 'preview' property in 'DocumentComponents'
- Inlined type definitions within module augmentation to avoid naming collision
- Ensured build process prefers correct type for '.d.ts' files
- Used type import to prevent runtime impact
@ricokahler ricokahler requested a review from a team as a code owner May 16, 2024 15:51
@ricokahler ricokahler requested review from jtpetty and removed request for a team May 16, 2024 15:51
Copy link

vercel bot commented May 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
performance-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 16, 2024 6:36pm
test-next-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 16, 2024 6:36pm
test-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 16, 2024 6:36pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Visit Preview May 16, 2024 6:36pm

Copy link
Contributor

No changes to documentation

Copy link
Contributor

github-actions bot commented May 16, 2024

Component Testing Report Updated May 16, 2024 6:41 PM (UTC)

File Status Duration Passed Skipped Failed
comments/CommentInput.spec.tsx ✅ Passed (Inspect) 34s 15 0 0
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 6s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ✅ Passed (Inspect) 26s 6 0 0
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx ✅ Passed (Inspect) 31s 11 7 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 14s 6 0 0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx ✅ Passed (Inspect) 35s 15 0 0
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 1m 16s 20 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ❌ Failed (Inspect) 1m 9s 17 0 1
formBuilder/inputs/PortableText/PresenceCursors.spec.tsx ✅ Passed (Inspect) 7s 3 9 0
formBuilder/inputs/PortableText/RangeDecoration.spec.tsx ✅ Passed (Inspect) 20s 9 0 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 14s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ✅ Passed (Inspect) 28s 12 0 0

Copy link
Member

@stipsan stipsan left a comment

Choose a reason for hiding this comment

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

Interesting, I wasn't aware we were using declare module '@sanity/types' { type techniques in the build of sanity 🤔

When it comes to generating .d.ts builds it's unfortunately the case that you often can accidentally overwrite exports silently, like the DocumentComponents case here.
In some cases you'll get a warning if exports collide, if they happen in an ambiguous way. In most cases it's viewed as intentional by rollup and the TypeScript compiler.

The way the rollup .d.ts build works is that we:

  1. First run tsc to generate .d.ts files in a temporary directory, as there is no other build tool that can generate definition files as well as tsc can.
  2. Since tsc isn't a bundler, you'll get one d.ts file for every .tsx and .ts in your src folder. We use these files as input with @microsoft/api-extractor.
  3. @microsoft/api-extractor doesn't support multiple entrypoints for a single npm package, so we run it once per entry (for sanity it means 16 times, once per exports in package.json, and once per bundles in package.config.ts), and it provides us with the neat, one .d.ts file per entry.

It's in step 3 that the overwrite happens and there isn't much we can do about it from the @sanity/pkg-utils.
The best guard I've seen is to use a setup like vitest' typecheck where you ensure that generated types on public interfaces work as they should and that you're not missing exports or doing unintentional overrides.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants