Skip to content

Conversation

@iChenLei
Copy link
Contributor

@iChenLei iChenLei commented Apr 16, 2021

What: To fix #2070 and fix #2373

Why: when using the new fragma jsxImportSource - to automatically add the new pragma definition to the top of the file page, like already is implemented with the old pragma /* @jsx jsx */

How: Add eslint rule support.

Checklist:

  • Documentation
  • Tests
  • Code complete
  • Changeset

jsx-runtime

react official blog -> https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html
babel support -> https://babeljs.io/docs/en/babel-plugin-transform-react-jsx#customizing-the-automatic-runtime-import

Add jsxImportSource option

The default custom-automatic-runtime is @emotion/react, more jsx-runtime support detail you can found in @emotion/react source code like follows
https://github.com/emotion-js/emotion/blob/main/packages/react/src/jsx-dev-runtime.js
https://github.com/emotion-js/emotion/blob/main/packages/react/src/jsx-runtime.js

- "@emotion/jsx-import": "error"
+ "@emotion/jsx-import": [2, "jsxImportSource"]
// .eslintrc.js
module.exports = {
  root: true,
  plugins: [ '@emotion'],
  rules: {
    "@emotion/jsx-import": [2, "jsxImportSource"]
   }
}

It works as follows:

emotion-eslint-02

Add jsxImportSource custom jsx-runtime option

- "@emotion/jsx-import": "error"
+ "@emotion/jsx-import": [2, { jsxImportSource: "customreact/jsx-runtime"  }]
// .eslintrc.js
module.exports = {
  root: true,
  plugins: [ '@emotion'],
  rules: {
    "@emotion/jsx-import": [2, { jsxImportSource: "customreact/jsx-runtime"  }]
   }
}

It works as follows:

emotion-eslint-01

/cc @Andarist @mitchellhamilton

@changeset-bot
Copy link

changeset-bot bot commented Apr 16, 2021

🦋 Changeset detected

Latest commit: 6ada9d9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@emotion/eslint-plugin Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codesandbox-ci
Copy link

codesandbox-ci bot commented Apr 16, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 6ada9d9:

Sandbox Source
Emotion Configuration

@codecov
Copy link

codecov bot commented Apr 16, 2021

Codecov Report

Merging #2353 (6ada9d9) into main (685bbec) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted Files Coverage Δ
packages/eslint-plugin/src/rules/jsx-import.js 97.77% <100.00%> (+1.05%) ⬆️

Copy link
Member

@emmatown emmatown left a comment

Choose a reason for hiding this comment

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

Could you change the config to be like this?

The default should be this:

{
  "runtime": "classic"
}

and to use the new runtime, it should be this

{
  "runtime": "automatic"
}

and with a custom importSource, it should be this:

{
  "runtime": "automatic",
  "importSource": "whatever"
}

@iChenLei
Copy link
Contributor Author

iChenLei commented Apr 24, 2021

After I changed the implemention:

{ "runtime": "automatic" }

emotion-eslint-02

{ "runtime": "automatic", "importSource": "whatever" }

emotion-eslint-o1

If your config is not oneOf [{ "runtime": "xx" }, { "runtime": "xx", "importSource": "yy" }], your code will be treated by the previous @emotion/eslint-plugin. jsxImportSource support is a special deal.

/cc @mitchellhamilton @Andarist Hi, maintainers, do you have a little time to review my PR? Thanks

@iChenLei iChenLei requested a review from emmatown April 24, 2021 07:38
@osdiab
Copy link
Contributor

osdiab commented May 10, 2021

Looking forward to using this!

@iChenLei iChenLei force-pushed the eslint-jsxImportSource branch from 53997a1 to 4bdbacd Compare June 30, 2021 06:16
Copy link

@ryanrabello ryanrabello left a comment

Choose a reason for hiding this comment

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

I'm not qualified to approve this PR. But I would love to be able to use the code in here

@iChenLei iChenLei requested a review from emmatown September 15, 2021 08:37
@avertin
Copy link

avertin commented Oct 14, 2021

I really appreciate this PR. For now, I've copied this as a local eslint rule for my project. Hopefully it can be integrated soon.

I noticed one difference between classic and automatic options that I was hoping to understand better. For classic, once the jsx import rule is satisfied, these additional rules are also satisfied ( 'jsx' is defined but never used - no-unused-vars AND 'React' must be in scope when using JSX react/react-in-jsx-scope). With automatic, after the jsx import rule is satisfied, these errors are still present. I'm new to eslint rules and I was wondering what accounts for this difference?

@emmatown
Copy link
Member

'jsx' is defined but never used - no-unused-vars

With the automatic runtime, that error is correct, it's unused and shouldn't be there.

'React' must be in scope when using JSX react/react-in-jsx-scope

That rule should be disabled when using the automatic runtime - https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/react-in-jsx-scope.md#when-not-to-use-it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@emotion/eslint-plugin: support @jsxImportSource Implement automatic adding of jsxImportSource pragma definition - eslint-plugin-emotion

5 participants