Skip to content

add hints to troubleshoot NextJS + TS issues. #2703

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.

Sign up for GitHub

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

Closed
justincorrigible wants to merge 1 commit into from
Closed

add hints to troubleshoot NextJS + TS issues. #2703

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions docs/typescript.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ When using our JSX factory, TypeScript only allows the `css` prop on components

### With the Babel plugin

[`@emotion/babel-plugin`](/docs/babel) is completely optional for TypeScript users. If you are not already using Babel, you probably shouldn't add it to your build tooling unless you truly need one of the features offered by `@emotion/babel-plugin`. On the other hand, there's no reason not to use `@emotion/babel-plugin` if you are already using Babel to transpile your TypeScript code.
[`@emotion/babel-plugin`](/docs/babel) is completely optional for TypeScript users. If you are not already using Babel, you probably shouldn't add it to your build tooling unless you truly need one of the features offered by this plugin. On the other hand, there's no reason not to use `@emotion/babel-plugin` if you are already using Babel to transpile your TypeScript code.

### With the old JSX transform

If you are unable to upgrade to the `react-jsx` transform, you will need to specify the JSX factory at the top of every file:
If you are unable to upgrade to the `react-jsx` transform (e.g. while using NextJS, which enforces `"jsx": "preserve"`), you may need to specify the JSX factory at the top of every file:
Copy link
Copy Markdown
Contributor

@srmagura srmagura Mar 31, 2022

Choose a reason for hiding this comment

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

@jsx jsx at the top of every file is not necessary to get Next, TypeScript, and Emotion to play nicely together, so I don't think we should recommend this to users. I rewrote the Emotion documentation site in Next & TypeScript (#2571) and got it to work with jsxImportSource in tsconfig.json and @emotion/babel-plugin — though I'm pretty sure the Babel plugin is not truly necessary here.

Copy link
Copy Markdown
Author

@justincorrigible justincorrigible Mar 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

thank you!
I also find that instruction to "add this everywhere" a bit odd, and wholeheartedly agree with you about it... however, as you may realise, it is not part of my changes. As you will notice in the existing docs, they already had that statement before my PR.

image

In fact, I'm suggesting to modify "will need" to "may need", so it doesn't sound as mandatory.
I tried to change as little as possible in the existing verbiage, but you're right: that line needn't be added everywhere, and I'd like to improve that too.

Naturally, I'm willing to improve this PR if you can advise better phrasing for those other sentences/words I didn't add/change. Any suggestions? I'm also ok closing this PR if your rewrite handles the issue I had.

Copy link
Copy Markdown
Author

@justincorrigible justincorrigible Mar 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

side note, you mention you got it to work with just the babel plugin and jsxImportSource? for some reason I wasn't able to get css props going correctly until I also added the /// declaration.
will give your rewrite a good read, and maybe you'll let me pick your brain if I cannot manage to make it work that way?

edit: just went through your rewrite (awesome amounts of work, btw), and could not find any other differences in config, apart from the NextJS version itself (I'm on ^12.1.2). I wonder if that (or any other package's version difference) is what didn't let the css props work? 🤷
Additionally, I see you're removing the pragma from all the docs, but I'm not sure I recall what you will recommend for people not using babel (or the emotion babel plugin, for that matter)?

Copy link
Copy Markdown
Contributor

@srmagura srmagura Mar 31, 2022

Choose a reason for hiding this comment

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

Thank you @caravinci 😊

  • The reason I commented on your change to this line is because, by mentioning Next.js in this way, it sounds like we are recommending @jsx jsx for all Next.js users.
  • I'm not sure why you needed the /// declaration to get TypeScript to recognize the css prop. I believe jsxImportSource in tsconfig.json is all that's needed to tell TypeScript about the extra prop.
  • I removed the @jsx pragma from all of the code samples because, some of those are live examples and the pragma was messing them up, and also I want the docs to be focused on the react-jsx transform which does not require the pragma.
  • I think the Emotion docs could be improved by adding an "Integration Guides" heading to the navigation menu on the right. It would be below the "Advanced" heading. The nav items under the new heading would be "Create React App / CodeSandbox", "Next.js", .etc. If you would like to work on this, I can create a new issue and reach out to Andarist for approval.

Copy link
Copy Markdown
Author

@justincorrigible justincorrigible Mar 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The reason I commented on your change to this line is because, by mentioning Next.js in this way, it sounds like we are recommending @jsx jsx for all Next.js users.

Though I'm not sure I would read it that way, I see what you mean and how others may indeed read it that way too. Those not using babel, in particular, may be led to misunderstand my changes.
Your comment makes sense to me now. Thanks for pointing that out 🙏

jsxImportSource in tsconfig.json is all that's needed to tell TypeScript about the extra prop

yeah, it seems reasonable to me too, but for some reason it didn't work in my project/setup. There may be something else going on here that's not obvious to me right now. I'll run some tests this weekend, see if I can at least figure out what the root issue is, and document it.

the Emotion docs could be improved by adding an "Integration Guides"

oh, that would be awesome, and very useful... especially as the NextJS team are actively working on better integration for their SWC. I will gladly help if possible, pending availability at my job, of course.

That said, this PR is not necessary.


```tsx
/** @jsx jsx */
Expand All @@ -61,15 +61,16 @@ import { jsx } from '@emotion/react'

As a result, you may be not able to use the shorthand syntax `<></>` for React fragments, but you can still use `<Fragment></Fragment>`. This is a limitation of the TypeScript compiler not being able to independently specify jsx pragma and jsxFrag pragma.

You can still use the css helper and pass the className yourself (ensure you are importing from the `@emotion/css` package, not `@emotion/react`).
You can still use the css helper and pass the className yourself (but ensure you are importing from the `@emotion/css` package, not `@emotion/react`).

```tsx
import { css } from '@emotion/css'

const el = <div className={css({ background: 'black' })} />
```

It's not possible to leverage `css` prop support being added conditionally based on the type of a rendered component when not using our jsx pragma or the `react-jsx` transform. If you use our pragma implicitly (for example when using our `@emotion/babel-preset-css-prop`) we have a special file that can be imported once to add support for the `css` prop globally, for all components. Use it like this:
Without using our JSX pragma or the `react-jsx` transform, it is not possible to leverage `css` props support being added conditionally, based on the type of a rendered component. However, if you use our pragma implicitly (for example when using our `@emotion/babel-preset-css-prop`) we have a special file that can be imported once anywhere in your app, which adds support for the `css` prop globally and for all components.
This may also be useful if you're encountering issues running a NextJS app. It can be used like this:

```ts
/// <reference types="@emotion/react/types/css-prop" />
Expand Down