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

React 17 default to react-jsx in tsconfig #5090

Closed
cyrus-za opened this issue Mar 22, 2021 · 12 comments · Fixed by #5131
Closed

React 17 default to react-jsx in tsconfig #5090

cyrus-za opened this issue Mar 22, 2021 · 12 comments · Fixed by #5131
Labels
outdated scope: react Issues related to React support for Nx type: feature

Comments

@cyrus-za
Copy link

With react 17 the import of React is not required to use jsx, but this is only possible if we got

  "compilerOptions": {
    "jsx": "react-jsx",
  }

in the tsconfig.json file.

Can the @nrwl/react generator check the version of react in package.json and if its >= 17.0.0 then set that in tsconfig?

@vsavkin vsavkin added the scope: react Issues related to React support for Nx label Mar 23, 2021
@ptrhck
Copy link

ptrhck commented Mar 24, 2021

Did you try that? I have added a lib with nx g @nrwl/react:lib shared-components, removed the React import and changed all tsconfig options to "react-jsx". Still, I am getting the error "React is not defined". May this be due to the .babelrc within the lib?

@jaysoo
Copy link
Member

jaysoo commented Mar 24, 2021

We can do this without checking since nx 10.4.0 already migrated workspaces to React 17. We just need to make sure we enable this in our @nrwl/react/babel preset.

Will do this for the next release

@ptrhck
Copy link

ptrhck commented Mar 24, 2021

Yes, the following works for me:

{
  "presets": [
    "@nrwl/web/babel",
    ["@babel/preset-react", { "useBuiltIns": true, "runtime": "automatic" }]
  ],
  "plugins": []
}

@jaysoo
Copy link
Member

jaysoo commented Mar 25, 2021

We'll need to update TS version as well to support react-jsx, so this PR addresses that #5116.

There's a separate PR incoming to update the generators and migrations.

Targetting for Nx 12 release.

@jaysoo jaysoo mentioned this issue Mar 26, 2021
@cyrus-za
Copy link
Author

cyrus-za commented Mar 26, 2021

Thanks @jaysoo looking forward to the next version.

Can we also add a eslint rule to not import the default export from react (seeing as that will eventually go away and be a breaking change)?

@puku0x
Copy link
Contributor

puku0x commented Mar 26, 2021

I found "importSource": "@emotion/react" is needed for Emotion users.

// .babelrc
{
  "presets": [
    [
      "@babel/preset-react",
      {
        "useBuiltIns": true,
        "runtime": "automatic",
        "importSource": "@emotion/react"
      }
    ]
  ],
  "plugins": ["@emotion/babel-plugin"]
}

@emotion/babel-preset-css-prop" should be replaced with @emotion/babel-plugin.

// tsconfig.json
{
  "extends": "../../tsconfig.base.json",
  "compilerOptions": {
    "jsx": "react-jsx",
    "allowJs": true,
    "esModuleInterop": true,
    "allowSyntheticDefaultImports": true
  },
  "files": [],
  "include": [],
  "references": [
    {
      "path": "./tsconfig.app.json"
    },
    {
      "path": "./tsconfig.spec.json"
    }
  ]
}

@jaysoo
Copy link
Member

jaysoo commented Mar 30, 2021

@cyrus-za

Can we also add a eslint rule to not import the default export from react (seeing as that will eventually go away and be a breaking change)?

Does such a rule exist for React?

@jaysoo
Copy link
Member

jaysoo commented Apr 1, 2021

@puku0x Emotion migration has been added https://github.com/nrwl/nx/pull/5131/files#diff-a477196d7726c04ca7eb1ec5b8dd71c4fe6d1f12e13515c9f040851456e2d744R1

New apps/libs will also generate with @emotion/babel-plugin instead of @emotion/babel-preset-css-prop.

@cyrus-za
Copy link
Author

cyrus-za commented Apr 1, 2021

@jaysoo I am not sure if there's a eslint rule for it right now. In fact the react-plugin-react/recommended preset for eslint has this turned on which I had to explicitly turn off in my .eslintrc.json

 rules: {
    'react/react-in-jsx-scope': 'off',
    'react/jsx-uses-react': 'off'
  }

I did however find this
jsx-eslint/eslint-plugin-react#2829

Maybe the rule can be written as part of the @nrwl/react package?

  TSQualifiedName(node) {
    if (node.left.name === 'React') {
      context.report(
        node,
        'Do not use React qualifier, use import deconstruction instead',
      );
    }
  },

Further reading:
jsx-eslint/eslint-plugin-react#2834
jsx-eslint/eslint-plugin-react#2928

@jaysoo
Copy link
Member

jaysoo commented Apr 1, 2021

@cyrus-za Thanks for the pointers. We have turned the two eslint rules off in our eslint-plugin-nx rules. See: https://github.com/nrwl/nx/pull/5131/files#diff-8f62961a5b32590af61aaee94cab47b8cf5ca114fbb7bd17a2b1974ebc2c78d3R30

I've opened an issue here to capture your eslint rule request. #5198

@cyrus-za
Copy link
Author

cyrus-za commented Apr 8, 2021

Thanks @jaysoo I'm upgrading to nx12 right now :)

@github-actions
Copy link

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated scope: react Issues related to React support for Nx type: feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants