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: return a jsx elemnet instead of string #8493

Merged
merged 2 commits into from
Jun 2, 2023

Conversation

BWizard06
Copy link
Contributor

The Success Component now returns a JSX Element instead of a string, which leads to an Error in the ArticlePage. Instead of return JSON.stringify(article) it is now return

{JSON.stringify(article)}
. With this change we don't get any error in the ArticlePage, where we use the ArticleCell

@Tobbe
Copy link
Member

Tobbe commented Jun 2, 2023

@BWizard06 Thanks for your PR!

If this isn't working, it sounds to me like a problem with our types, rather than the code in the tutorial

export const Success = ({ article }: CellSuccessProps<ArticleQuery>) => {
  return JSON.stringify(article)
}

string is a valid child in React, so JSON.stringify() should be allowed here.

What's the exact error you're getting?

@BWizard06
Copy link
Contributor Author

Hello @Tobbe . I am getting following error if i don't return it in a div:
'ArticleCell' cannot be used as a JSX component.
Its return type 'string' is not a valid JSX element.
I get this error in the ArticlePage when I just return it without a div

@Tobbe
Copy link
Member

Tobbe commented Jun 2, 2023

How are you using <ArticleCell />? Can you show me some example code? Or even better, can you link to a github repo that shows the error?

@BWizard06
Copy link
Contributor Author

Yeah sure. I have made a github repo. In this repo the error still remains as you had wished. https://github.com/BWizard06/redwoodblog/blob/master/web/src/pages/ArticlePage/ArticlePage.tsx

@Tobbe
Copy link
Member

Tobbe commented Jun 2, 2023

Thank you, that's really helpful.

I can confirm that I'm seeing the same thing

image

image

But there are no errors inside the cell itself

image

If you click through into the types for ArticleCell you'll end up in .redwood/types/mirror/web/src/components/ArticleCell/index.d.ts where you see this

image

And I think this is wrong.

You can see the type Redwood uses for the Success component here

CellSuccess extends keyof JSX.IntrinsicElements | JSXElementConstructor<any>,

I'm not sure yet how they should change, but I think React.ReactNode should be allowed. React.ReactElement is too narrow.

@BWizard06 Do you want to try to look into how to fix the types? No pressure, just an open invitation 🙂 If you don't want to or don't have time to try to fix it, I or someone else from the RW team will do it instead.

@BWizard06
Copy link
Contributor Author

Honestly I would really like to but I am pretty new and don't quite understand how I could change the type of the article, so that it accepts the type string. I think this is the key error:
Type 'string' is not assignable to type 'ReactElement<any, any>'
But another question, when I put the return into a div it works, why can't we just put it in a div, because all other exports (Loading, empty...) return a div too?

@replay-io
Copy link

replay-io bot commented Jun 2, 2023

@Tobbe
Copy link
Member

Tobbe commented Jun 2, 2023

But another question, when I put the return into a div it works, why can't we just put it in a div, because all other exports (Loading, empty...) return a div too?

Yeah, let's do that for now. And then I can fix our types later 🙂

@Tobbe Tobbe added the release:docs This PR only updates docs label Jun 2, 2023
@Tobbe Tobbe enabled auto-merge (squash) June 2, 2023 17:27
@Tobbe Tobbe merged commit 221ccb3 into redwoodjs:main Jun 2, 2023
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone Jun 2, 2023
@Tobbe
Copy link
Member

Tobbe commented Jun 2, 2023

Thanks for your PR @BWizard06. Much appreciated 👍

@jtoar jtoar modified the milestones: next-release, docs Jun 2, 2023
jtoar pushed a commit that referenced this pull request Jun 2, 2023
@jtoar jtoar modified the milestones: docs, next-release Jun 2, 2023
@BWizard06
Copy link
Contributor Author

Glad I could help😊

@jtoar jtoar modified the milestones: next-release, v5.3.0 Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:docs This PR only updates docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants