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

When JSX is used, automatically import 'react' appropriately #603

Closed
wants to merge 1 commit into from

Conversation

jayphelps
Copy link
Contributor

@jayphelps jayphelps commented Sep 8, 2016

Now if they use JSX, it will transparently import react and correctly reference the right createElement(). No more awkward import React from 'react';.

Before
import React from 'react';

const App = () => (
  <div>hello world</div>
);

export default App;
var _react = __webpack_require__(35);

var _react2 = _interopRequireDefault(_react);

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

var App = function App() {
  return (0, _react.createElement)(
    'div',
    null,
    'hello world'
  );
};

exports.default = App;
After
const App = () => (
  <div>hello world</div>
);

export default App;
var _react = __webpack_require__(35);

var App = function App() {
  return (0, _react.createElement)(
    "div",
    null,
    "hello world"
  );
};

exports.default = App;

Anyone I show this ability to so instantly says "omg why didn't I use this earlier", so I imagine it is a welcome addition; but perhaps some will say it's "too much magic"...but let's be real, JSX is magic.

I could not find a test suite for create-react-app; I assume there isn't currently a test suite for the cli/scripts?

@gaearon
Copy link
Contributor

gaearon commented Sep 8, 2016

Thanks for the PR!

There is an end-to-end test in tasks/e2e.sh, it passed.

I think that, however, we won't take this. I'm worried that while this is great for a single project, it hurts the ecosystem. People will copy and paste their project code into StackOverflow answers or Gists but those won't work without that setting. Unlike CRA users, those who paste those files might not have a linter so they won't understand what's causing the issue.

If we change how this works we should do it consistently across the ecosystem.

@just-boris
Copy link
Contributor

@gaearon's point is pretty fair, but now the question is: can we fix the ecosystem?
I agree, that import React into module when this is not mentioned directly is wrong, because now every code-linting tool should assume that React is actually used however it looks like doesn't. It is actually a hack, that should not exist.

So, can we start to move the whole ecosystem towards the right direction at some point? This project seems like a good starter.

@gaearon
Copy link
Contributor

gaearon commented Sep 27, 2016

People use React in variety of situations including global UMD builds. The plugin doesn’t consider this use case (it emits an import but maybe the user wants a global access).

If you think it should be the default behavior of React JSX transform, please raise an issue with babel-preset-react in Babel repo, and let’s discuss it there. I don’t have enough context about the original decision to do it this way to give you a good reply.

@sylvainbannier
Copy link

@jayphelps maybe you could add a doc on how to add this feature (if this is possible without ejecting) ?

@just-boris
Copy link
Contributor

@sylvainbannier no way. It is not possible to add any babel plugin without ejecting

@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants