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

Type Checking on Typescript project finds errors with an unchanged project #5104

Closed
GaeaKat opened this issue Apr 9, 2022 · 10 comments · Fixed by #5112
Closed

Type Checking on Typescript project finds errors with an unchanged project #5104

GaeaKat opened this issue Apr 9, 2022 · 10 comments · Fixed by #5112
Labels
bug/confirmed We have confirmed this is a bug topic/typescript

Comments

@GaeaKat
Copy link

GaeaKat commented Apr 9, 2022

Expected Behavior

Type checking on a unchanged fresh typescript project to return 0 errors

Current Behavior

Type checking on an unchanged fresh typescript project
returns

(node:19764) ExperimentalWarning: stream/web is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
  ✔ Generating the Prisma client...
  ✔ Generating types
src/App.tsx:10:4 - error TS2786: 'FatalErrorBoundary' cannot be used as a JSX component.
  Its instance type 'FatalErrorBoundary' is not a valid JSX element.
    The types returned by 'render()' are incompatible between these types.
      Type 'import("/Users/katrinaknight/Projects/redwood/test-app/node_modules/@types/react/index").ReactNode' is not assignable to type 'import("/Users/katrinaknight/Projects/redwood/test-app/node_modules/@types/react-dom/node_modules/@types/react/index").ReactNode'.
        Type '{}' is not assignable to type 'ReactNode'.

10   <FatalErrorBoundary page={FatalErrorPage}>
      ~~~~~~~~~~~~~~~~~~


Found 1 error in src/App.tsx:10

Steps to Reproduce

  1. yarn create redwood-app --ts test-app
  2. cd test-app
  3. yarn rw type-check

Why this effects things

This doesn't effect me too much right now, as long as it can be ignored and will show other errors in addition to this one, but it is a minor issue if I try to later do CI based on the type-checking.

@jtoar jtoar added this to Triage Apr 9, 2022
@jtoar jtoar moved this to Needs triage in Triage Apr 9, 2022
@jtoar jtoar added bug/confirmed We have confirmed this is a bug topic/typescript labels Apr 10, 2022
@jtoar
Copy link
Contributor

jtoar commented Apr 10, 2022

Thanks @katrinaas! I can reproduce this and the clue is this line here:

Type 'import("/Users/katrinaknight/Projects/redwood/test-app/node_modules/@types/react/index").ReactNode' is not assignable to type 'import("/Users/katrinaknight/Projects/redwood/test-app/node_modules/@types/react-dom/node_modules/@types/react/index").ReactNode'.

It looks like we're getting the type from two places. I looked at the latter (@types/react-dom/node_modules/@types/react/index) and it turns out those type definitions are for React 18. I'll continue investigating!

@Tobbe
Copy link
Member

Tobbe commented Apr 10, 2022

Maybe relevant: facebook/react#24304

So I don't know why we have react-18 types. But the issue here is that in the react 17 types ReactNode includes {}. It was removed in the react 18 types. So assigning from react 17 ReactNode to react 18 ReactNode is not possible (but the other way around would be fine)

React 17:

type ReactFragment = {} | Iterable<ReactNode>;
type ReactNode = ReactChild | ReactFragment | ReactPortal | boolean | null | undefined;

React 18:

type ReactFragment = Iterable<ReactNode>;
type ReactNode = ReactChild | ReactFragment | ReactPortal | boolean | null | undefined;

This is only a problem in RW Apps, not the framework itself. For the framework we still get react 17 typings for both react and react-dom

image

Is it react-syntax-highlighter that's the problem?

A few other things worth looking into, from a project's yarn.lock

"@testing-library/react@npm:12.1.4":
  version: 12.1.4
  resolution: "@testing-library/react@npm:12.1.4"
  dependencies:
    "@babel/runtime": ^7.12.5
    "@testing-library/dom": ^8.0.0
    "@types/react-dom": "*"
"@types/react-dom@npm:*":
  version: 18.0.0
  resolution: "@types/react-dom@npm:18.0.0"
  dependencies:
    "@types/react": "*"
"@types/react-syntax-highlighter@npm:11.0.5":
  version: 11.0.5
  resolution: "@types/react-syntax-highlighter@npm:11.0.5"
  dependencies:
    "@types/react": "*"

I found out about the -R flag for yarn why

image


So. Workaround for now:

Add this to your project's root package.json:

  "resolutions": {
    "@types/react": "17.0.40"
  }

Repository owner moved this from Needs triage to Done in Triage Apr 11, 2022
@thedavidprice
Copy link
Contributor

If I understand correctly, the problem here is how tsc module resolution is handling the @types/react package as a dependency of:

  • @types/react-dom
  • @types/react-syntax-highlighter

Is this something that can be handled via tsconfig.json?

@thedavidprice
Copy link
Contributor

Related Issue now open on DefinitelyTyped repo:
microsoft/DefinitelyTyped-tools#433

@cq-ubaid-khan
Copy link

The equivalent of resolution for npm is overrides see documentation https://docs.npmjs.com/cli/v8/configuring-npm/package-json#overrides

@tiff0000
Copy link

tiff0000 commented Feb 6, 2023

any resolution to this issue?

@jtoar
Copy link
Contributor

jtoar commented Feb 6, 2023

@tiff0000 if I recall correctly the original issue was resolved a while ago from @type/react's side, but I just tried reproducing it and I see other, node-related type errors. Is this what you're referring to or are you still seeing the original issue?

> yarn rw type-check
✔ Generating the Prisma client...
✔ Generating types
src/pages/FatalErrorPage/FatalErrorPage.tsx:12:5 - error TS2591: Cannot find name 'process'. Do you need to install type definitions for node? Try `npm i --save-dev @types/node` and then add 'node' to the types field in your tsconfig.

12 if (process.env.NODE_ENV === 'development') {
       ~~~~~~~

src/pages/FatalErrorPage/FatalErrorPage.tsx:14:5 - error TS2591: Cannot find name 'require'. Do you need to install type definitions for node? Try `npm i --save-dev @types/node` and then add 'node' to the types field in your tsconfig.

14     require('@redwoodjs/web/dist/components/DevFatalErrorPage').DevFatalErrorPage
       ~~~~~~~


Found 2 errors in the same file, starting at: src/pages/FatalErrorPage/FatalErrorPage.tsx:12

@tiff0000
Copy link

tiff0000 commented Feb 6, 2023

Hey @jtoar ! Thanks for the reply, I resolved the issue by upgrading types/react from 17 to 18.

@snurfer0
Copy link

snurfer0 commented Dec 4, 2023

having the same issue with "@types/react": "18.2.13",

@Tobbe
Copy link
Member

Tobbe commented Dec 4, 2023

@snurfer0 Can you please be a little bit more specific about exactly what errors you're seeing?
Please also include the output of yarn rw info
Thanks! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/confirmed We have confirmed this is a bug topic/typescript
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

8 participants