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 TypeScript error: Cannot find name 'Fragment'. #1615

Merged
merged 1 commit into from
Aug 28, 2021

Conversation

jablko
Copy link
Contributor

@jablko jablko commented Aug 19, 2021

Add import { Fragment } from "react", missed in #1514 ❤️

@vercel
Copy link

vercel bot commented Aug 19, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/mdx/mdx/3sdwbvUUm5AvQLrD8BHELX7ege9m
✅ Preview: https://mdx-git-fork-jablko-patch-1-mdx.vercel.app

@vercel vercel bot temporarily deployed to Preview August 19, 2021 17:25 Inactive
@@ -4,6 +4,7 @@ import {
Context,
Consumer,
ComponentType,
Fragment,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixes:

~/mdx/packages/react $ yarn test
Error: /home/nottheoilrig/mdx/packages/react/types/index.d.ts:68:20
ERROR: 68:20  expect  [email protected] compile error: 
Cannot find name 'Fragment'.

@@ -3,7 +3,6 @@
"module": "commonjs",
"lib": ["dom", "es6"],
"strict": true,
"skipLibCheck": true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Stop omitting TypeScript errors from automated tests: sed --in-place /skipLibCheck/d **/tsconfig.json

Copy link
Member

Choose a reason for hiding this comment

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

This option enabled makes ts ignoring errors in .d.ts, right? But what if there are some errors from node_modules?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This option enabled makes ts ignoring errors in .d.ts, right?

Yup.

But what if there are some errors from node_modules?

If a way exists to verify our .d.ts without also checking declarations in node_modules, I don't know it ... 🤔 Could we use our dependencies/devDependencies and semvers to keep broken types out of node_modules? If that works, it would probably be the friendliest thing to our downstream types consumers?

Copy link
Member

Choose a reason for hiding this comment

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

OK, maybe we can raise a feature request for TypeScript?

Copy link
Contributor Author

@jablko jablko Aug 28, 2021

Choose a reason for hiding this comment

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

I ran into an issue: Since dropping skipLibChecks we're checking the following .d.ts:

  • our explicit node_modules dependencies
  • every node_modules/@types declaration 😂

#1622 adds "types": [], to at least only check our explicit dependencies.

@@ -1,4 +1,4 @@
// TypeScript Version: 3.4
// TypeScript Version: 3.5
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixes:

~/mdx/packages/preact $ yarn test
Error: Errors in [email protected] for external dependencies:
../../../node_modules/preact/src/jsx.d.ts(310,6): error TS2304: Cannot find name 'Omit'.

Preact depends on TypeScript 3.5: preactjs/preact#2581

@vercel vercel bot temporarily deployed to Preview August 19, 2021 18:46 Inactive
Copy link
Member

@ChristianMurphy ChristianMurphy left a comment

Choose a reason for hiding this comment

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

Thanks @jablko!

@ChristianMurphy ChristianMurphy merged commit 3a965db into mdx-js:main Aug 28, 2021
@ChristianMurphy ChristianMurphy added the ☂️ area/types This affects typings label Aug 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
☂️ area/types This affects typings
Development

Successfully merging this pull request may close these issues.

3 participants