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

Proposal: [react][breaking] Only .js files should contain JSX. #985

Closed

Conversation

KevinGrandon
Copy link
Contributor

Opening this for comments and discussion. I would like to suggest only using JSX inside of .js files. Here is a short rationale for doing so:

  • Most modern projects are transpiling code with tools that support JSX. In this case it makes sense to keep everything together inside a single .js file.
  • Cleaner import and require statements. For .js files I can import the file by its name, instead of having to include the .jsx file. E.g., require('./MyComponent'); vs require('./MyComponent.jsx');
  • While JSX is not ecmascript, it is transpiled JS, in the same vein that flow and stage 0 babel transforms are. There's no reason to differentiate.

Additional discussion: facebook/react#3582 (comment)

@ljharb
Copy link
Collaborator

ljharb commented Aug 2, 2016

Thanks, but no. At Airbnb we believe that only JavaScript should ever go in .js files. JSX is not standard JS, and is not likely to ever be - if that changes, then our stance may as well.

You never need extensions whether it's .js or .jsx - and in fact, our linter config requires (or will soon require) that you omit file extensions in all cases.

The reason to differentiate is the same reason we only use stage 3 and above proposals: if it's not standard, it is not javascript.

In that comment on the react repo, "What people are actually doing… that's may not match." - which is indeed the case. It's far more common to put JSX in .jsx files in my experience, and regardless, that's what this guide will continue to encourage.

@battaile
Copy link

battaile commented Aug 2, 2016

Not intended to argue with your decision here, but just to share a differing view on this point:

It's far more common to put JSX in .jsx files in my experience

Although true in the past, I think this is starting to change, for reasons pointed out here: facebook/create-react-app#87 (comment)

This is also something I'm seeing when looking at other people's new projects, but certainly could be selection bias.

@ljharb
Copy link
Collaborator

ljharb commented Aug 2, 2016

The reality is that whether it's common isn't really relevant. JS is JS. .js should only be JS. The answer to the slippery slope fallacy of "well what about .flow.es6.jsx.js???" is "pick one non-standard extension for your org/project and use that, and only put JS in .js".

We've chosen .jsx.

@battaile
Copy link

battaile commented Aug 2, 2016

The reality is that whether it's common isn't really relevant.

Oh ok.

@merlinstardust
Copy link

our linter config requires (or will soon require) that you omit file extensions in all cases.

@ljharb do you have an estimate as to when this might be the case?
Also, do import statements automatically search for files that have the same name if there's no extension or does this need to be configured by webpack or a similar tool?

@ljharb
Copy link
Collaborator

ljharb commented Aug 3, 2016

@merlinpatt it's blocked on import-js/eslint-plugin-import#390 - so, as soon as that's implemented.

Yes, import works just like require (and since ES modules don't actually exist yet, it's always just transpiled to require anyways), so it automatically locates extensions for you.

@nathanhleung
Copy link
Contributor

nathanhleung commented Aug 7, 2016

@merlinpatt .jsx isn't picked up by the webpack require, you'd have to add this to your config so it resolves for it:

module.exports = {
  ...
  resolve: ['', '.js', '.jsx']
  ...
}

@gaearon
Copy link

gaearon commented Aug 11, 2016

The reality is that whether it's common isn't really relevant. JS is JS. .js should only be JS. The answer to the slippery slope fallacy of "well what about .flow.es6.jsx.js???" is "pick one non-standard extension for your org/project and use that, and only put JS in .js".

If you are already using something like JSX, keeping JS separate from JSX hurts ergonomics in my experience. It feels silly to have to rename a single file just because you need a single line of JSX there.

A similar argument can be made for Flow annotations. If you already decided on using Flow, it feels silly to ask team members to rename .js to .jsx (or whatever) just to add a single annotation file, thus “penalizing” Flow additions with extra work and loss of commit history.

When you add a non-standard syntax extension, you usually do this because it provides enough convenience to your project to justify the non-standardness. Now that you’ve added it, having to do extra work bookkeeping and renaming files one by one is frustrating and defeats the purpose.

From this follows that if you decided to adopt a non-standard extension, you might as well allow it globally in your project anyway. Therefore there should be just one file extension. The remaining question is which one.

.jsx could work for this, but as I already said, people don’t agree on syntax extensions. Some want JSX, others want stage 0, others want Flow, some want Sweet.js, etc. In big companies you often also have custom AST transforms for different purposes. And this set often changes during project lifetime.

So I guess we could call it .myCompanysIdeaOfJSAtThisMomentInTime but .js seems simpler.

(I would like to add that we are in a unique point in time where one of the most popular languages in the world is being actively extended through transpilers. Our usual strategies and rules of thumb might not apply in this situation. Whether it’s good or bad, time will tell.)

@MilosRasic
Copy link

It feels silly to have to rename a single file just because you need a single line of JSX there.

How often does this happen though? Usually you'll have files that will clearly contain JSX (components) and those that don't (app state code, helpers, etc).

I've found that using Redux, in contrast to a more es6 class inheritance based Flux implementation, will result in having reducers with the same name as component, for example reducer in somePage.js and a SomePage.jsx component. Having different extensions helps differentiate the files without looking at the whole path.

@AndrewIngram
Copy link

I used to have the same viewpoint as @ljharb, and this is the exact pattern I followed for my first 2 years with React. But recently (in the last 6 months) I came to the conclusion that the costs of ideological purity didn't justify the downsides (as elegantly described by @gaearon).

@gaearon
Copy link

gaearon commented Aug 11, 2016

How often does this happen though? Usually you'll have files that will clearly contain JSX (components) and those that don't (app state code, helpers, etc).

For Flow (or stage 0 extensions), it happens pretty much every day.
For JSX, not as often, but still happens, and having to think about this is annoying.

@tleunen
Copy link

tleunen commented Aug 11, 2016

Afaik, they said they've chosen .jsx for the reasons they mentioned. So yep we can argue on why someone prefers using .js or even .react.js (like the one Facebook seems to use), but at Airbnb, they decided to use .jsx. It's their styleguide after all...

And if for any reason you have to change the extension of a file (what I think you should not do during a project), you can do it in 2 steps in order to keep the git history (1: change the extension; 2: modify the file the way you want).

In the end, the extension doesn't matter. Use whatever you want to use.

@RavenHursT
Copy link

So absolutely annoyed by the “But AirBnB/Twitter/Facebook/Google/Paypal/Netflix/Mom&Pop.com do it that way!” fallacious argument that seems to be ​_constantly_​ tossed about in software engineering..

No one company (or even a handful) has exclusive knowledge on solutions. If one is a serious professional, they'll make decisions based off of actual knowledge and experience, not just because "everyone else does it that way"

https://yourlogicalfallacyis.com/appeal-to-authority

@ljharb
Copy link
Collaborator

ljharb commented Aug 11, 2016

@RavenHursT I think you misread - nobody is saying "X is right because $bigCompany chooses it", @tleunen was just pointing out that since this is Airbnb's style guide, Airbnb will put in it what works for Airbnb.

That said, that shouldn't be used to shut down this healthy discussion, but it's useful context if by discussing you're hoping to effect a change in this part of Airbnb's style guide.

@RavenHursT
Copy link

No one is attempting to "shut down" the discussion.. Simply making a point is all. I understand this is AirBnb's style guide. Unfortunately/fortunately many have taken this guide on as their own as well, based simply on "but AirBnb uses it" and along w/ it come the justifications for what is/is not in the guide. It's those justifications that I'm referring to, grounded in the fallacy.

@bradencanderson
Copy link

@ljharb:

...since this is Airbnb's style guide, Airbnb will put in it what works for Airbnb.

That said, that shouldn't be used to shut down this healthy discussion, but it's useful context if by discussing you're hoping to effect a change in this part of Airbnb's style guide.

Can you clarify a bit more what this will mean in practice? Does it mean controversial community PRs will only be discussed and not actually seriously considered? Does whatever Airbnb does trump community suggestions, generally?

Obviously this is Airbnb's config and what Airbnb says goes, but I wonder what you think the balance ought to be.

@ljharb
Copy link
Collaborator

ljharb commented Aug 11, 2016

@bradencanderson oh no, not at all. Airbnb has adapted its internal style based on repo discussions many times. If a persuasive case is made here, we could change this as well - but we haven't as of yet been persuaded.

@hshoff
Copy link
Member

hshoff commented Aug 12, 2016

@bradencanderson my favorite part of our style guide being open source is how it continues to get better with feedback from the community, so we welcome, enjoy, and encourage discussions like this. We've had 251 contributors since we open sourced our guide back in 2013. I guesstimate 20 or so are/were Airbnb employees. It's mostly reasonable... and sometimes opinionated. Currently we do find ourselves with opinions on semicolons and file extensions 😄

On to the topic at hand. There are great points on both sides of this discussion and some existential thoughts around what JS is, which is quite fun. @gaearon's follow up thoughts on Twitter are lovely (h/t Sgt Pepper): https://twitter.com/dan_abramov/status/763712190272667648

What would you think if I sang out of tune?
Would you stand up and walk out on me?

So the grand experiment continues! We've got an opinion that works for us at Airbnb right now, but we'll report back if things get unwieldy.

Picture yourself on a train in a station
With plasticine porters with looking glass ties
Suddenly someone is there at the turnstile
The girl with kaleidoscope eyes

As long as an opinion stays consistent throughout a project/codebase/team we've found efficiency increases even if there isn't consensus.jsx

Yes, I get by with a little help from my friends
With a little help from my friends

Thanks to everyone that has participated in this discussion so far!

@colbycheeze
Copy link

colbycheeze commented Aug 12, 2016

I think it is silly to name ALL files containing jsx with .jsx but I do have a convention of naming the actual component itself as .jsx

This allows easy fuzzy searching if I have multiple files named "component" within modules, test, story, route etc...also with file-icons plugin in Atom it gives a pretty little blue jsx icon to represent it, and overall just makes it easy to spot which file name is actually the component itself

image

@alex-wilmer
Copy link

How often does this happen though? Usually you'll have files that will clearly contain JSX (components) and those that don't (app state code, helpers, etc).

I'm finding it happens more and more often. In 'model' type objects:

const userTable = [
  {
    id: 'name',
    th: <th>Name</th>,
    td: data => <td>{data.name}</td>,
  }
]

..or dispatching a component in a thunk

(dispatch, getState) => {
  const value = getState().foo
  dispatch(toggleModal(<div>{value}</div>))
}

It's very liberating once you realize components are just function calls and you can put them anywhere. Having to switch file extensions just for this purpose is rather annoying.

@RustyF
Copy link

RustyF commented Aug 14, 2016

@alex-wilmer although it's nice to have the freedom of placing jsx tags anywhere, I would usually keep them separated from the js. Architecturally it just feels better to me but also because I know at some point react might be replaced with something else. So for me the jsx extension is telling me the code is in a different "layer" to my js code.

@mlangenberg
Copy link

mlangenberg commented Aug 14, 2016

So everyone should configure Sublime Text or Atom to handle .js as JSX?

Update

By the way, this is already a solved problem. Back in the days in Rails used to name HTML with Embedded Ruby .rhtml. But when people started to use different preprocessors they changed to filenames to reflect this chain.

So index.rhtml became index.html.erb. index.rjs (ruby generated js) became index.js.rjs and index.rxml became index.xml.builder.

In this naming convention, a plain JavaScript file that needs to be preprocessed by flow, then by jsx would be called file.js.jsx.flow.

@alex-wilmer
Copy link

So everyone should configure Sublime Text or Atom to handle .js as JSX?

@mlangenberg I have a gut feeling a large majority of users building React apps use the Babel syntax extension for JS, which handles JSX by default.

@aight8
Copy link

aight8 commented Jan 7, 2017

jsx is the file extensions are source files for babel. it should be industry standard. totally disagree to use js extension for everything else. would NEVER do that.

@oshalygin
Copy link
Contributor

oshalygin commented Jan 11, 2017

I've adopted a similar approach to @colbycheeze on various projects over the past couple years. Components get the .jsx extension while everything else gets .js. That way the test files which HAVE JSX syntax are still just JavaScript files at the end of the day and are used as such.

The .jsx extension helps easily identify the components within a folder.

@ljharb
Copy link
Collaborator

ljharb commented Jan 11, 2017

.component.js or .component.jsx would work for that just fine, without having to annihilate the meaning of file extensions :-)

@dorsywang
Copy link

The distinction between .js and .jsx files was used to mean something before Babel, but nowadays the concept and meaning are different.We can't be conservative, follow the time.

@ljharb
Copy link
Collaborator

ljharb commented Jun 19, 2017

@dorsywang i'm not aware there was jsx before babel. The distinction remains forever relevant, and will be even more relevant as node.js ships ES Modules support that requires the file extension be .mjs.

@AndrewIngram
Copy link

@ljharb there was JSX before Babel. Non-Facebook use of React with JSX predates Babel and even 6to5 (what Babel used to be called), React provided its own compiler which included support for JSX and a small number of other ES6 features.

@ljharb
Copy link
Collaborator

ljharb commented Jun 20, 2017

@AndrewIngram ah, thanks - that was a long time ago :-)

The advent of babel still didn't erase the purpose of file extensions; otherwise we'd all just use extensionless files instead of .js :-).

@markspolakovs
Copy link

TypeScript has .ts and .tsx, however there's a technical reason: TypeScript uses <foo>bar as type assertion syntax, which can confuse the parser, so this syntax is disallowed in .tsx files.

Seems like there is no reason like this for vanilla JS, though.

Just my $0.02.

@ljharb
Copy link
Collaborator

ljharb commented Aug 5, 2017

@markspolakovs yes, that's precisely because different parsing goals are required for the two files, so TypeScript has done the proper thing and assigned each parsing goal its own file extension. That's what we're doing here with JS vs "JS plus extra stuff".

@markspolakovs
Copy link

@ljharb Thing is, JSX doesn't need different parsing goals. TypeScript uses the same syntax for two different things, that's why it needs the disambiguation. JSX syntax isn't used anywhere else in JavaScript, neither is Flow for example. The syntax is enabled by your configuration (.babelrc/.flowconfig/etc), however there is no ambiguity about the syntax.

@ljharb
Copy link
Collaborator

ljharb commented Aug 6, 2017

Of course it does; jsx isn't valid JS. "parsing goal" means "different rules", which doesn't require that there be conflicts.

@markspolakovs
Copy link

I meant different parsing goals in the same project. It's because of the fact that TS uses the same syntax for two different things that it needs this "selector" by file extension, but since there isn't anything else in JS that uses JSX syntax, it's impossible for there to be two files with different parsing goals (in terms of JSX syntax) in the same project, so you don't need an explicit JSX specifier on your files, it can be global.

@ljharb
Copy link
Collaborator

ljharb commented Aug 6, 2017

@markspolakovs yes, I know what you mean, but the point of file extensions is to answer the question "across the entire universe of parseable things, not just this project, what's the parsing goal of this file?" The fact that you can parse a .js file and decide, at parse time, if you're parsing it as JSX, or as normal JS, doesn't make the extension meaningless; it's just a property of the combination of two similar parsing goals.

@markspolakovs
Copy link

I see your point about the entire universe of parseable things, but I wonder how often that's relevant. The only instances I can think of off the top of my head are IDEs and GitHub's syntax highlighting, and those seem to be able to detect JSX even inside of vanilla .js files. Therefore I think putting the JSX in the extension is redundant.

lmorchard added a commit to lmorchard/testpilot that referenced this pull request Sep 18, 2017
lmorchard added a commit to mozilla/testpilot that referenced this pull request Sep 22, 2017
* Tweaks to support component directory bundles

- Storybook now also finds stories in frontend/src/app/**/stories.js

- `npm run test` now also finds tests in frontend/src/app/**/tests.js

- New /static/app/app.js.css stylesheet built from Sass styles imported
  by components

- Hacks to ignore .scss imports when using component modules outside
  Webpack for static page build & tests

- Update flowconfig to ignore .scss imports

- Small eslint upgrade so CLI matches gulp-eslint

* Reorganize ExperimentRowCard component into bundle directory

* Reorganize UpdateList component into bundle directory

* Begin to extract IncompatibleAddons component from ExperimentPage container

* Rename stories.jsx -> stories.js; See also: airbnb/javascript#985 (comment)

* Quick & dirty revision to Storybook docs to incorporate new component bundle proposal

Issue #2807
@mraak
Copy link

mraak commented Jan 10, 2018

Every programming language uses the same extension for different versions of the language. I've never seen .py2, .py3, .java6, etc. although a lot of it is incompatible between the versions of the interpreter/compiler/transpiler. Same goes with .js with a small specialty of having some unofficial albeit very popular transpilers that are de facto industry standards and their features should be supported as standard functionality.

.jsx is dead, long live .js

@clayrisser
Copy link

https://github.com/facebook/create-react-app/releases/tag/v0.4.1
React does not recommend jsx anymore

@RavenHursT
Copy link

LOLz! ☝️ this is awesome.

@ljharb
Copy link
Collaborator

ljharb commented Feb 13, 2018

CRA’s recommendation isn’t relevant (or new).

This thread is long enough; I’m going to lock it. .js (and soon, .mjs) is for JavaScript; “not JavaScript” must be something else - we’ve chosen .jsx.

@airbnb airbnb locked and limited conversation to collaborators Feb 13, 2018
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.