facebook / create-react-app

Set up a modern web app by running one command.
https://create-react-app.dev
MIT License
102.84k stars 26.89k forks source link

General feedback #11

Closed gaearon closed 8 years ago

gaearon commented 8 years ago

Let’s use this thread for a general discussion.

Does the flow feel good? Did we pick the right plugins, presets, and loaders? Do you see areas for improvement?

This project is in a very early stage so there’s plenty of all hanging fruit if you’d like to make React experience better.

Thanks for taking a look at this project!

gaearon commented 8 years ago

cc @insin @mxstbr @eanplatter @ericclemmons, thank you so much for your time & ideas

ericclemmons commented 8 years ago

Should we encourage or support something like nvm? Or will we specify a minimum version of Node that we can test against to ensure consistent behavior/performance?

gaearon commented 8 years ago

I'd add node >= 4 to engines. The CLI script actually already checks against this field (we took it from RN).

gaearon commented 8 years ago

We definitely want to support n / nvm.

eanplatter commented 8 years ago

I'd be interested to hear what people thought about eliminating 100% of the boilerplate code. We tried to do this we enclave but made up for it with a small amount of configuration. (telling enclave where to find the index.html / React entry point).

@gaearon I know you mentioned not wanting to have the user configure anything, so I am thinking more of an implicit kind of thing.

While it might not be totally pragmatic to hide the index.html file, it might make sense to just let the user know that they need to create a Main.js file in the root of their app. (similar to something like Elm).

While boilerplate is a helpful way to show a new user how to get started, it can be hard for beginners to know what they can touch.

Also, sometimes when a user is using something with boilerplate, it can make community resources (like tutorials) a bit harder to grok because they're having to dance around code written by the tool.

I'm willing to be swayed either way :P but in my experience I've found that beginners appreciate the ability to understand all the code written in their application.

keyz commented 8 years ago

I was about to ask you if we want multiple scaffolds/boilerplates, but I'm not sure if that aligns with the design decision here. Maintaining multiple scaffolds is also an overhead. As a solution, I thought about this a while ago and I imagined a npm init-like interactive cli initialization experience -- something like:

Do you want to use CSS Modules? (Y/n)

My motivation behind this is, CSS Modules is popularly adopted and a lot of people would want to use it in their project. But if I'll need to "graduate" in order to add the config, then it feels more like a yet-another boilerplate again. Do you think this fits with our design goal?

gaearon commented 8 years ago

@gaearon I know you mentioned not wanting to have the user configure anything, so I am thinking more of an implicit kind of thing.

While it might not be totally pragmatic to hide the index.html file, it might make sense to just let the user know that they need to create a Main.js file in the root of their app. (similar to something like Elm).

Isn't this what we do? The location and names of index.html and src/index.js are conventional, you can't change them. The HTML file was added because people really want to customize it (for example meta tags or google analytics) but it doesn't let users change anything related to bundling.

I was about to ask you if we want multiple scaffolds/boilerplates, but I'm not sure if that aligns with the design decision here

To me, this is an explicit non-goal. People have attempted to do this, but this creates a combinatorial explosion of different incompatible tools and approaches. Beginners won't know what to choose, and advanced users will be frustrated there is not enough choice.

My motivation behind this is, CSS Modules is popularly adopted and a lot of people would want to use it in their project.

Are they good enough and easy enough to explain that we can make them a default for everyone? Every other single tool that we use, in my opinion, passes this test, but CSS Modules, as much as I like them, don't yet, in my opinion.

This is why we don't include them, and this is our philosophy in general. If we can't recommend something to everyone using React, or at least consider it a sane default while keeping in mind that for many people this is the first exposure to React, we don't add it.

Perhaps in a year CSS Modules will be obvious to everyone and we will get many requests from beginners to support them. Then we ship a codemod as yet another script that ports everyone's projects to use CSS Modules, and they become our default recommendation. I totally see this happening but not today. We need to pick our battles.

insin commented 8 years ago

Likely first issues:

"Where's the hot reloading?"

One of the issues opened on the same day reactpack was posted on Show HN was related to not using the default ESLint presets they chose - how easy is it to disable it or use your own with the current setup? Code style can be a controversial topic, even if people have specifically asked for more opinions.

gaearon commented 8 years ago

We might eventually add hot reloading when it is stable enough but I don't think it should be a part of the official starter if it's hacky right now.

I do expect some people to ask for custom lint configuration but I'd rather not do it. Give in just a little, and you'll end up with a lot of configuration. It will be hard to draw the line where to say "yes" and where to say "no". React has never been opinionated so I think we have a right to release one very opinionated tool.

eanplatter commented 8 years ago

I do expect some people to ask for custom lint configuration but I'd rather not do it.

What is your thinking behind adding linting to begin with? It might make sense for it to be lint agnostic.

mxstbr commented 8 years ago

It's a nicer dev experience, and the react community is pretty set on using airbnb. Imo that's fine!

gaearon commented 8 years ago

In my experience linting catches many mistakes. Beginners who are just getting started often don’t bother to configure a linter, and suffer from errors caused by simple typos.

Having control over linting also gives us the ability to introduce custom rules and potentially add API deprecations.

eanplatter commented 8 years ago

👍 is there a lint script in the users new project?

gaearon commented 8 years ago

No, we do linting as part of both npm start and npm run build. It’s done via eslint-loader.

So the user sees something like this when they save a file:

screen shot 2016-07-18 at 13 56 25
gaearon commented 8 years ago

If the project doesn’t crash, those warnings also show up in the console:

screen shot 2016-07-18 at 13 57 31
eanplatter commented 8 years ago

I guess they can always eject if they want to configure things themselves.

lacker commented 8 years ago

I found the linting frustrating when I tried starting here and following along on other tutorials that don't follow the linting rules. Some of the rules like no-var and semi are in conflict with a lot of our own example code. What do you think about having lint hidden behind something like npm run lint?

In my experience of asking people, most folks do not use a linter. And most folks who say they use "airbnb lint" have actually customized it a bit.

lacker commented 8 years ago

So right after I ran create-react-app, it had a file:/// dependency in the package.json. Is that supposed to happen when this is live, or is that an artifact of running it from npm run create-react-app? If it actually creates a project with a dependency on the creation tool which lives outside the repo, then I can't collaborate with other people on it.

lacker commented 8 years ago

Also, I ejected pretty soon. Why not just eject right away? - I did not see any advantage of using the un-ejected version. The tool is already creating some boilerplate for me, it might as well create more.

lacker commented 8 years ago

Also, how about rendering to a specific element rather than doing const rootEl = document.createElement('div'); document.body.appendChild(rootEl); render(<App />, rootEl);

That way you don't need to muck around with js code if you need to do something like add some custom html to your app.

gaearon commented 8 years ago

So right after I ran create-react-app, it had a file:/// dependency in the package.json. Is that supposed to happen when this is live, or is that an artifact of running it from npm run create-react-app? If it actually creates a project with a dependency on the creation tool which lives outside the repo, then I can't collaborate with other people on it.

No, this is because packages are not on npm yet. In the “normal” npm flow, you’d get a regular dependency.

gaearon commented 8 years ago

Also, I ejected pretty soon. Why not just eject right away? - I did not see any advantage of using the un-ejected version. The tool is already creating some boilerplate for me, it might as well create more.

The number one complaint I heard about “JavaScript fatigue” is that there are tons of dependencies in package.json and you aren’t sure how/when to update them because they are often incompatible, each requires maintaining its own config, releases breaking changes, etc.

https://twitter.com/thomasfuchs/status/708675139253174273

Here is a good example of an issue like this. Somebody was trying to follow a “React for Beginners” starter guide but updated Babel and got a weird error: https://github.com/babel/babel-loader/issues/255. If you hunt around, you’ll find hundreds of issues like this across different boilerplate projects, React, Redux, Babel, ESLint, etc.

If you’re comfortable configuring ESLint, Babel, and Webpack yourself, you can indeed eject right away. This is the level of utility already provided by the popular boilerplate projects. However, if you’re not used to those tools, having a bunch of complicated configs in your folder is very intimidating. I’m trying to provide a better experience for people who are just getting started with React and don’t want to customize anything—rather, they want somebody to hold their hand while they’re writing their first component. And providing the escape hatch turns it from a toy into a real tool.

Another intention is that we keep improving the default set of plugins over time. For example, we might switch to Webpack 2, or Babel 7, or ESLint 3, or whatever, at our own pace, at the same time ensuring they all work great together. This is something a user can’t easily do if they’re on their own unless they have an extensive knowledge of all those tools.

gaearon commented 8 years ago

Also, how about rendering to a specific element rather than doing const rootEl = document.createElement('div'); document.body.appendChild(rootEl); render(, rootEl);

That way you don't need to muck around with js code if you need to do something like add some custom html to your app.

I’m not sure what you mean. What kind of custom HTML? Why not do it in a React component instead? To be clear, this tool is targeted towards people creating greenfield single-page apps. React already has a good integration story; we just want to fix the “start from scratch” story.

We can change it for sure, but I’d like to better understand the use case you have in mind.

lacker commented 8 years ago

I was thinking adding other libraries like analytics, where you often have noobs following some instructions of how to modify their html. It's a bit simpler to figure out what's happening if there is a div already in index.html that your app gets rendered to.

lacker commented 8 years ago

Another intention is that we keep improving the default set of plugins over time. For example, we might switch to Webpack 2, or Babel 7, or ESLint 3, or whatever, at our own pace, at the same time ensuring they all work great together.

So the idea is that someone would be able to just upgrade their create-react-app-scripts dependency, and then they could get more modern stuff? That seems cool. If you can't upgrade this set of scripts, though, then it seems equivalent to saying "heres some boilerplate, just never update it".

gaearon commented 8 years ago

I found the linting frustrating when I tried starting here and following along on other tutorials that don't follow the linting rules. Some of the rules like no-var and semi are in conflict with a lot of our own example code. What do you think about having lint hidden behind something like npm run lint?

To be honest I would rather choose our own subset of rules that is more flexible about code style than remove linting altogether from the flow. In my experience of replying to issues on React and Redux repo, people often do mistakes that would be caught by a linter. A separate npm run lint wouldn’t help them because they wouldn’t run it.

In my experience of asking people, most folks do not use a linter. And most folks who say they use "airbnb lint" have actually customized it a bit.

I think there is a middle ground here: beginners and people who didn’t bother to use a linter because it is too complicated to set it up (e.g. ESLint didn’t work with ES6 until recently, babel-eslint integration was often buggy while it was necessary, etc).

Maybe my bet is off. I don’t know. But I feel like it’s better to start enforcing the lint, and later loosen it up, than start relaxed and later tighten it up. We can teach people that it is an opinionated tool, and this is what we think is the best way to start getting up to speed with React.

We use vars in the docs because our docs assume ES5. This is a different and separate issue: we do intend to update them, but we need a plausible “getting started” experience that includes ES6 for this. So it’s a bit of a chicken-and-egg problem.

I was thinking adding other libraries like analytics, where you often have noobs following some instructions of how to modify their html. It's a bit simpler to figure out what's happening if there is a div already in index.html that your app gets rendered to.

Perhaps. I thought the comment in index.html helps, does it not?

So the idea is that someone would be able to just upgrade their create-react-app-scripts dependency, and then they could get more modern stuff? That seems cool. If you can't upgrade this set of scripts, though, then it seems equivalent to saying "heres some boilerplate, just never update it".

Yes, the idea is that we release versions once in a while that track what’s going on in the ecosystem and put the best stable things together.

lacker commented 8 years ago

To be honest I would rather choose our own subset of rules that is more flexible about code style than remove linting altogether from the flow.

Mmm that sounds good to me. I tried using the airbnb style on a sample app and found it was too strict for what I wanted - in particular I ended up disabling

"rules": { "arrow-body-style": 0, "new-cap": 0, "no-console": 0, "no-param-reassign": 0, "no-undef": 0, "no-useless-constructor": 0, "no-use-before-define": 0, "react/prefer-stateless-function": 0, },

Like, if we tell noobs they shouldn't use console.log, they will become sad.

I really like the idea that we can update these tools over time and thus not become dependent on eslint specifically.

Also I think it would be nice to try to use similar language presets for the default apps created by React and React Native - it'll just make it easier for anyone building or documenting a cross-environment library.

gaearon commented 8 years ago

Would you like to send a PR with the ESLint rule subset you find reasonable?

lacker commented 8 years ago

Sure - not fair of me to gripe without offering to fix it ;-)

vjeux commented 8 years ago

I was going to say, we already have Facebook eslintrc on fbjs but it seems like every single of our open source project is using a different variation:

kentcdodds commented 8 years ago

While you're at it, maybe you could add: "semi": [2, "never"] :trollface: Sorry, someone had to say it. (But if that happened, I'd be pleased)

gaearon commented 8 years ago

I think our guideline should be: let’s ship ESLint warnings that help find bugs or avoid bugs in the future.

lacker commented 8 years ago

Playing around, I find myself often deleting some of the autogenerated files, because I don't want some subset of App.js, App.css, index.css, logo.png. What do you think about simplifying the autogenerated stuff to just index.html and index.js in the main folder? (No src.) The pattern where you have your index files at the root and then other stuff is in a folder is how React Native typically works, for example. (E.g. https://github.com/fbsamples/f8app ) And that pattern seems fairly common for React apps too. E.g. https://github.com/reactjs/redux/tree/master/examples/todomvc That way people can decide if they want a "src" folder or what.

gaearon commented 8 years ago

My intention was to bootstrap a project that shows:

Those are the most common questions I see about Webpack-based setups, and it is not obvious at all that you need to import CSS until you see it. So I think it’s important to explain it to people who never worked with Webpack.

If we don’t provide these, people will try adding images in local directory and expecting them to be available by paths like src='./logo.png', they will add CSS to index.html without realizing we already solve autoprefixing for them, and they will import components with named imports (import { App } from './App') while using default exports and be frustrated that this doesn’t work.

How do you think we could solve those problems without frustrating experienced users?

gaearon commented 8 years ago

FWIW I’m not completely opposed to CLI flags. create-react-app my-app --template empty is fine in my book because this lets us add more official examples later to get people started with something they can run and tweak.

eanplatter commented 8 years ago

I can't help but agree, it's easy to forget how valuable it is to see first hand how to do something like reference a CSS file in a React component.

Keeping the boilerplate as minimal as possible (like it currently is) is important. Perhaps some inline comments to help dispel any questions about the generates code itself?

gaearon commented 8 years ago

Let’s continue this discussion (on templates) in https://github.com/facebookincubator/create-react-app/issues/24.

lacker commented 8 years ago

So I am actually one of these webpack noobs, I haven't forgotten what it's like ;-)

Before using this tool I never included css and images with an import statement the way it works here. I'll admit it was helpful. I didn't realize there was "autoprefixing" going on, though. How exactly does that work? Is it just using the stuff in https://github.com/postcss/autoprefixer ? Perhaps just a brief comment explaining that would be helpful somewhere in the code.

Also, it was unclear to me whether the "src" directory is special in some way. I have used some webpack configs where you have to put everything in a certain directory, and some where it does not work that way. Is there any requirement besides the starting point being src/index.js? Like do other image / css files have to also be in the src directory? Personally I often prefer to not call the directory "src" because if you e.g. end up making a rails backend, your ruby code is also "src" and so it's confusing to call just one of your languages "src". Or for React Native.

gaearon commented 8 years ago

Before using this tool I never included css and images with an import statement the way it works here. I'll admit it was helpful. I didn't realize there was "autoprefixing" going on, though. How exactly does that work? Is it just using the stuff in https://github.com/postcss/autoprefixer ? Perhaps just a brief comment explaining that would be helpful somewhere in the code.

I think we need a “How To” document. It would explain the available features: ES modules, importing CSS, autoprefixing. It doesn’t need to be long, just needs to answer 10 common questions in a practical way.

Is there any requirement besides the starting point being src/index.js?

Having index.html and src/index.js is the only requirement. (Ideally we should check for their existence and crash with a descriptive message if either was not found.)

Like do other image / css files have to also be in the src directory?

All modules have to be there, src is where the code of your application is. We may want to introduce other top-level folders later, e.g. vendor for third party assets you want to reference directly from .html. This would be an escape hatch for scripts that aren’t on npm for some reason. We wouldn’t want webpack to look at those, so having a folder just for modules is useful.

Personally I often prefer to not call the directory "src" because if you e.g. end up making a rails backend, your ruby code is also "src" and so it's confusing to call just one of your languages "src". Or for React Native.

React Native is a very special case. I would probably go into direction of sharing code with React Native, so you’d still put your modules into src but “fork” components with build time flags. Other top-level directories could be used for RN-specific purposes (e.g. Xcode project). But I’m not very aware of RN project structure.

I don’t exclude that one day this will be ready to work with RN out of the box. We can rethink directory structure on that day, when we understand the constraints a little better.

As for Rails or Node, my intention is for the client to be treated as a separate “app” from its backend. So I would assume something like this:

my-awesome-app
  backend
    src
    config
    tasks
    email-templates
  frontend
    src
    vendor
    whatever
insin commented 8 years ago

Do you have a longer-term scope in mind?

Assuming the first release is an opinionated starter, is a longer-term goal is to become the react-cli people keep asking for?

What I'd really like to see is something which handles the core development setup (and owns configuring the DX adds like HMR, auto-install) and can install opinionated project setups which build on top of that with their own configuration, starting you out from a skeleton, providing generators for whichever opinions it has, with codemods to keep code up to date as things evolve. It would also need to be configurable on top of that default where the opinions can be flexible.

The goal would be that the core development setup and opinionated project setup mean upgrading when dependencies change is just bumping a version number, with codemods when you need them as long as you've stuck with the opinions.

The way react-project provided glue code as a dependency rather than boilerplate was also really interesting to me.

gaearon commented 8 years ago

Assuming the first release is an opinionated starter, is a longer-term goal is to become the react-cli people keep asking for?

It is hard to say. I definitely don't want any plugin system (which is what Ember CLI is famous for). The intention is to free user from configuration and incompatibilities. It we replace this with another configuration system or another addon system with its own issues, we defeat the point. (No offense to Ember.)

What you describe sounds ambitious but it is very hard to pull off. I want to start small and see how it goes.

I don't think code generators are as valuable as people say they are. If you need to generate code it means your libraries aren't expressive enough. Stuff like "add new component" — what's wrong with just writing a function by hand? The only frustrating thing is remembering to import React when you use JSX but this would better be solved on the IDE level or with an "autofix" system.

I am definitely interested in "initial" templates as described in #24. They are not as flexible as code generators but are much less pain to maintain and create. These are just "examples" of integration with libraries.

So we will see how it goes. It's hard to say in advance. For now we want to solve the build configuration problem. If it is adequately solved, we can consider solving other problems (such as integrating runtime libraries) but we need to be careful not to inflate the scope of the project too much.

insin commented 8 years ago

I definitely don't want any plugin system (which is what Ember CLI is famous for)

I don't think React even needs that in the same way Ember does - once Webpack is set up right, you just import and go instead of having to make stuff available to a template context. Largely the same thing with testing; once your tools are set up right, the actual tests are just import and use.

Stuff like "add new component" — what's wrong with just writing a function by hand?

I agree on generators for trivial things, I'm thinking more of when you have some conventions which are working for you, you need to do something which cuts across multiple modules and dependencies and you'd like to make it reusable across multiple projects. For example if I were to write an add section <path> <name> generator for the conventions I'm using in my main React project at work right now, it would:

Something like the above is a lot of work, but it mostly happens outside the global CLI and the module which solves the base build configuration problem, and it needs a way to hook into them or be hooked into by them. Just raising it now as the current template this project has is exactly as it should be IMO - here's your first component, the build stuff is handled for you, let rip - so it doesn't need these hooks.

gaearon commented 8 years ago

I think most of these points should be solved on the library level, not code generation level. For example:

Add a new definition to the appropriate place in src/routes/root.js which will code split the route component (so far, so Ember)

This is a problem with having global configuration. Imagine React Router worked in a different “100% React” way and nested routes would be implemented as nested components, without centralized config. Then this wouldn’t be an issue because you’d just add a new route right in the component you are working on. (Something like this may or may not be in development right now—not by us though. I won’t spoil it further. 😉 )

"Does this section need a reducer?" - if so, take a name, create src/reducers/.js as a duck module with the same layout as our other reducers, and also configure loading and injecting this reducer when the route first loads (and handling hot reloading setup), and create a suitable unit test skeleton.

Again, this seems like a problem with React state model. Ideally I think we should make React more reducer-friendly so that it’s easy to describe local state with a reducer, and move a reducer across the components. Then you wouldn’t have to jump between the files.

To be clear, I’m not saying what you suggest is not valid. Somebody could implement a generator like this as another devDependency, and maybe we’d want to merge it in eventually. But I think the need for code generation is usually symptomatic of abstractions that are not expressive enough, and we might want to fix that instead.

mxstbr commented 8 years ago

That's exactly what we do for our react-boilerplate generators – e.g. when you generate a container, it adds a bunch of files:

index.js     # Component itself, can either be function or class
actions.js
constants.js
reducer.js
test/        # The tests for all of those
  actions.test.js
  reducer.test.js
  component.test.js

Doing this manually every time is quite tedious, so having that automated and pre-populated with the correct data saves quite a bit of time!

gaearon commented 8 years ago

Redux is not a default way of using React. Quite the opposite, it is a very opinionated way of using it. So we definitely won’t include any code generation for reducers / action creators / etc here. We should improve React instead to address the shortcomings that make people go for Redux.

gaearon commented 8 years ago

That said nothing prevents somebody from creating react-redux-generator that would be another devDependency and generate this stuff.

eanplatter commented 8 years ago

Code generation can also fuel misinformation, it makes it easy for a user to be ignorant of the code in their app, this is especially an issue for beginners.

I think the need for code generation is usually symptomatic of abstractions that are not expressive enough, and we might want to fix that instead.

This is kind of the crux of it ☝️

If there are issues with React's usability we don't want to sweep them under the rug by just writing the code for you.

mxstbr commented 8 years ago

Didn't mean to imply those generators should live here, sorry!

gaearon commented 8 years ago

Haha no offence taken. 😄

montogeek commented 8 years ago

What would be the testing library? Enzyme is a nice candidate :)