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

some pragmatic features #636

Closed
ccorcos opened this issue Sep 12, 2016 · 35 comments
Closed

some pragmatic features #636

ccorcos opened this issue Sep 12, 2016 · 35 comments
Milestone

Comments

@ccorcos
Copy link

ccorcos commented Sep 12, 2016

Hey there, I'm considering dropping my existing build system in favor of create-react-app, but there are a few features that I would need. At some point, I'll likely need to fork, but until then, I'm curious if any of these features appeal to you enough to open up a PR.

  • absolute imports

In a large codebase, relative paths can get really annoying and make it hard to determine where files are being used. I'd like to suggest aliasing the project name to the root of the project. That way you can write something like import Env from 'create-react-app/config/env'. And if you want to find out where this file is used, its a simple find-all query for the file path.

  • babel-polyfill

looks like this project isnt using the babel-polyfill. any reason you opted to use a variety of other polyfills instead?

  • stage-0 preset

similarly, is the any reason this project is using a bunch of babel plugins rather than just using the stage-0 preset?

  • postcss variables / imports

What is the preferred method of sharing CSS variables amongst files? My current solution is to @import a variables.css file that doesnt generate any css so long as autoprefixer is prefixing for a browser that doesnt support css variables. I'm not sure this is the best solution since eventually this will result in duplicate css once browsers support css variables. But I dont want to rely on magic globals either.

  • stylelint

any interest in adding stylelint as a postcss plugin?

  • configurable environments / feature flags

I'd like to be able to run the application from the cli specifying the NODE_ENV and/or feature flags that use the define plugin. Something like NODE_ENV="replay" npm start which would let me start up the app in development mode with my replay devtool so I can run through various pre-recorded redux scenes.

  • deploy elsewhere

We're deploying our stuff to S3. Given this is a "zero-config" build tool, I'm not sure how adding an s3 deploy script would work.

  • how to build component library package?

Suppose I have a bunch of shared React components. How would I build this package so that I can publish it on NPM and require it from another project? The fact that babel isnt parsing node_modules means that we need to build a distribution file. But we also shouldnt be minifying or hashing the filenames, and we also dont need an index.html file. Any ideas how we could outfit this tool to work for React component libraries?

  • zero-configuration (configurable) build tool

this all leads me to thinking that maybe the right way to go is to have a configurable build tool with some zero-configuration defaults. how to I set the eslint config, flowtype config, stylelint config, babel config? I dont want to rely on a .babelrc or something that I have to copy into all of my projects. Ideally, I could create a package called chets-build-system which depends on create-react-app-core and configures it with my custom configs. the create-react-app package can simply have a set of sane defaults but doesnt necessarily have all the feature I want...

@tbillington
Copy link

Just suggesting some things from my (a user) point of view.

Absolute imports

#276 #253

Custom environments

https://github.com/facebookincubator/create-react-app/blob/ef94b0561d5afb9b50b905fa5cd3f94e965c69c0/template/README.md#adding-custom-environment-variables

Library

Not sure this is the aim of this project, 'create-react-app' rather than 'create-react-lib'. It might be best to look for other boilerplates ? Otherwise you could eject and modify the webpack build to a library.

config

Search the issues, it's been talked about before.

@insin
Copy link
Contributor

insin commented Sep 13, 2016

how to build component library package?

#423

zero-configuration (configurable) build tool

This isn't currently a goal of create-react-app.

Check out some of the alternatives if this is what you're after - the following allow configuration to different degrees:

@gaearon
Copy link
Contributor

gaearon commented Sep 13, 2016

Thanks for raising this and sharing your thoughts!

absolute imports

I’m open to adding this but I’m not sure that blindly allowing everything in src to be referenced absolutely is a great way of doing it. Particularly I saw confusion from beginners who used boilerplates with absolute imports, named their folders react or redux, and started getting conflicts with node_modules.

We already support running npm start, npm run build, or npm test, with NODE_PATH environment variable, which sort of lets you do that explicitly. Like NODE_PATH=./src npm start. But I can see that this might feel a bit frustrating.

Can you propose a solution to this in a separate issue?

looks like this project isnt using the babel-polyfill. any reason you opted to use a variety of other polyfills instead?

It’s quite heavy. People already blame React for “being large”. We don’t want to prescribe a large polyfill when many users opt to not use runtime ES2015 features, or use them at their own risk. We encourage people to add granular polyfills for features they actually use instead.

We opted to add a Promise and fetch polyfills because we consider them to be fundamental to doing async in React apps these days. Also people often choose bad Promise polyfills that don’t log unhandled rejections, and then blame React for “swallowing” their errors, which is not true. So we prefer to have control over that. On the contrary, features like Array.from() are not fundamental, so we’d rather give control over them to the user.

similarly, is the any reason this project is using a bunch of babel plugins rather than just using the stage-0 preset?

Absolutely. We only enable transforms that are:

  • Either stable;
  • Or actively used at Facebook.

This is why class-properties and async/await are enabled but decorators are not.

Our reasoning for this is also simple. We think some of these experimental features provide enough value that we’re willing to enable them even at the cost of the churn later. However we never want to expose our users to this churn.

If class-properties doesn’t work out, we’ll write a codemod at Facebook and share it with the community, so that people can migrate away from it. We know it’ll work well because we’ll also use it internally for thousands of components at Facebook.

However if something like decorators doesn’t work out, or changes syntax, we know we won’t be able to write a good codemod for it because we just don’t use this proposal for our components. So we’d essentially leave users relying on it in the dark, which is in direct opposition to Create React App philosophy.

To sum up: we only enable plugins that work great together, and that we commit to supporting through codemods in case of changes or deprecations.

What is the preferred method of sharing CSS variables amongst files? My current solution is to @import a variables.css file that doesnt generate any css so long as autoprefixer is prefixing for a browser that doesnt support css variables. I'm not sure this is the best solution since eventually this will result in duplicate css once browsers support css variables. But I dont want to rely on magic globals either.

There is an issue about this: #130. Unfortunately nobody came up with a PR yet, would you like to give it a try? As elsewhere, we are open to enabling features that are low-risk, high-value, relatively stable, and are future standard compatible.

any interest in adding stylelint as a postcss plugin?

Can you raise a separate issue about this?

I'd like to be able to run the application from the cli specifying the NODE_ENV and/or feature flags that use the define plugin. Something like NODE_ENV="replay" npm start which would let me start up the app in development mode with my replay devtool so I can run through various pre-recorded redux scenes.

https://github.com/facebookincubator/create-react-app/blob/master/template/README.md#adding-custom-environment-variables

We're deploying our stuff to S3. Given this is a "zero-config" build tool, I'm not sure how adding an s3 deploy script would work.

It would work by literally adding a deploy entry to scripts in your package.json that does what you need. For example it could look like this:

  "deploy": "npm run build && node ./my-scripts/deploy.js"

You could also extract it into a package.

how to build component library package?

Not currently supported, but experiments are welcome. We can’t solve all use cases right now, let’s figure out apps first 😉 .


I’ll close this issue because it’s a bit too meta, and in my experience such issues don’t really move projects forward. I hope my replies show future work directions for each of these items, and you are welcome to submit PRs or file new issues for the actionable items that you care about.

@gaearon gaearon closed this as completed Sep 13, 2016
@AlicanC
Copy link

AlicanC commented Sep 13, 2016

absolute imports

I have made the terrible decision of giving all general-purpose directories their own alias in a few projects. I ended up with multiple aliases like components => src/components, helpers => src/helpers, etc. and they became hard to maintain since I had to duplicate all of them for Flow too. Currently I only use one alias, app => src, and it helps a lot.

Of course the name "app" is taken and can cause conflicts. A better name (something that can't be an npm package name?) can be chosen for this.

@cecilemuller
Copy link

cecilemuller commented Sep 13, 2016

If you want simple paths like "components/something", how about using local modules ?
The name "node_modules" is not just for thirdparty code from npm (npm just uses it because it's how Node resolves paths). Then you have clean paths without needing folder aliases in every tool of the stack, non-cross-platform hacks, or extra dependencies :)

@gaearon
Copy link
Contributor

gaearon commented Sep 13, 2016

I could get behind src/modules.

@ericclemmons
Copy link
Contributor

I prefer a separate "/lib":

https://medium.com/@ericclemmons/dogfooding-your-open-source-projects-9e6dc1e7d1c8

It helps within our codebases to have a "staging" area outside of "src". In the past, you'd often have a "utils" folder with generic utilities.

The intent was to make clear that these should become open-sourced libraries.

@insin
Copy link
Contributor

insin commented Sep 13, 2016

While it's still magical, aliasing src to src feels like the least magical solution to avoiding ../ path traversal to me, as the resulting top-down import paths match what you're seeing in your editor.

// From somewhere deep under src/routes/
import LoadingButton from 'src/components/LoadingButton'
import Spinner from 'src/components/Spinner'
import {createActionDispatchers} from 'src/utils'

I usually group these together as a separate import block to try to make that even clearer (Node.js built-in imports → node_modules/ imports → top-down imports → local imports).

Is there some sample functionality we could try different suggestions out with to see what directory structures and imports end up looking like?

@AlicanC
Copy link

AlicanC commented Sep 13, 2016

@gaearon how is that supposed to help? For example, why would a component be in src/components instead of src/modules/components or vice versa?

If we are crazy for asking an easy way to refer to src, then it must be considered bat-shizzle crazy to use @providesModule. How is it okay to do import X from 'X'; (which gives you no freaking idea where "X" is and what its purpose is) and not okay to do import X from 'app/components/X'; (which shows you where it is and also that it's a React component)?

Aliasing src is just a better @providesModule.

@gaearon
Copy link
Contributor

gaearon commented Sep 13, 2016

Another option: just use own package name when importing.
Resolves relative to project folder.

To access my-app/src/stuff/index.js, import from my-app/src/stuff.

The src part is redundant so instead we can encourage people to put "modules" into the root folder. We would also enable Babel on the whole project directory (except node_modules).

Then, to access my-app/stuff/index.js you would import from my-app/stuff.

This way your entry module just happens to be called src but you can add other modules, and absolute paths are obvious (they include the app name) and work the same way.

And you can't accidentally import the wrong thing because your app cannot be dependency of itself (npm won't allow it). So it can't ever be a dependency.

@gaearon
Copy link
Contributor

gaearon commented Sep 13, 2016

This option would also provide a reasonable solution for #67 because paths would be consistent between browser console and code.

@mlrawlings
Copy link

mlrawlings commented Sep 13, 2016

@ericclemmons I really like the idea of using lib/ to stage modules. That makes a lot of sense. You could also use app-module-path for the node side of things.

@gaearon However, it seems the original request is not so much for a place to put local modules as much as a way to require files using a path based at the root of the project. I've always thought it would be nice to be able to require the current project by name (search up the directory tree for a package.json and if your require matches the name entry it uses that as the starting point).

For libraries, this would be nice as if you were developing a lib foo, you could require('foo') in any examples and tests within your project and they would work, but also be portable.

I also like the idea of ~ being aliased to the current project. So you could require('~/src/foo.js') and although this would conflict with the unix-style home directory alias, I'm willing to bet you're doing something you shouldn't be doing if you're trying to require something from your home directory. This may be taking things too far, but I thought I'd throw it out there.

@gaearon
Copy link
Contributor

gaearon commented Sep 13, 2016

However, it seems the original request is not so much for a place to put local modules as much as a way to require files using a path based at the root of the project

That's exactly what I meant by "local modules". A folder where absolute paths get resolved, in addition to node_modules (hence the name).

@AlicanC
Copy link

AlicanC commented Sep 13, 2016

encourage people to put "modules" into the root folder

Letting the app use files outside of src is a big no-no. You will have files that contain secrets outside of src. Letting your app access outside of src is just bad practice. The app must be contained in src and never access anything outside of it.

@gaearon
Copy link
Contributor

gaearon commented Sep 13, 2016

In case of my second proposal, enabling own project to be required by name solves exactly the problem of absolute paths. The only difference is that you have to prefix path with your app's name but that's the prince of safety and obviousness. Anyone who wants less typing can call their package app.

@gaearon
Copy link
Contributor

gaearon commented Sep 13, 2016

Well, those files can only end up in output if somebody imports them on purpose. We don't copy any folders except the build products produced by the bundler, which will only include things that import each other.

@AlicanC
Copy link

AlicanC commented Sep 13, 2016

People will import them on purpose. You are at Facebook and working with the best developers in the world. In my case, I need simple and strict rules so people (including myself) don't end up doing something silly.

The current rule is "don't touch anything outside of src" and it works. If you make the rule "you can touch some things outside of src", a developer can easily assume it's also okay to import a file which contains something useful for the app, but also contains secrets.

I can't expect everyone to be full-stack and know what API and development secrets are and I can't trust tree-shaking for secrets to be removed.

@AlicanC
Copy link

AlicanC commented Sep 13, 2016

Also, this isn't only about leaking secrets. Currently everyone knows what they are supposed to touch and that's always in src.

For example, if you are supposed to implement designs, you touch src/components and nothing else. There is no root, no src/modules/components, no nothing. It's dead simple.

@ccorcos
Copy link
Author

ccorcos commented Sep 13, 2016

Thanks for all the feedback. I'll look into these further and open up separate PRs.

@gaearon
Copy link
Contributor

gaearon commented Sep 13, 2016

Another option is to document usage of something like https://www.npmjs.com/package/linklocal.

@ccorcos
Copy link
Author

ccorcos commented Sep 13, 2016

I think an ideal solution for me is if require.resolve resolves the package.json name as the directory of the package.json file. For example:

mkdir cool-app
cd cool-app
npm init -y
echo "module.exports = { name: 'chet' }" > defs.js
echo "console.log('hello' + require('cool-app/defs').name)" > index.js
node index.js
> hello chet

Thus we're able to reference the package itself just like any other package :)

Anyone know where the require.resolve code is? I've been looking around and cant find it.

@cecilemuller
Copy link

cecilemuller commented Sep 13, 2016

Just in case to clarify, when I said local modules earlier, I meant src/node_modules, not src/modules.

Here's an example: note how neither webpack, flow, babel or mocha need aliases, NODE_PATH or such to find "component/MyImage" for example.

(and if the file was named index.js instead of MyImage.js, it wouldn't even need a package.json, but I prefer giving a more descriptive name than index.js, easier to see what file is what when I have many tabs open in the IDE 😃 )

As for module.resolve, those links might help:

Substack had also made a module that implements the same logic:
https://www.npmjs.com/package/resolve

@gaearon
Copy link
Contributor

gaearon commented Sep 13, 2016

Let’s keep this open just so we don’t lose this discussion, and revisit this at some point.

@gaearon gaearon reopened this Sep 13, 2016
@gaearon gaearon added this to the 1.0.0 milestone Sep 13, 2016
@ccorcos
Copy link
Author

ccorcos commented Sep 14, 2016

@cecilemuller do you know where the actual node.js source code is that implements require.resolve?

@cesarandreu
Copy link

cesarandreu commented Sep 14, 2016

My general strategy is to create an app folder in the project root, add a package.json, and symlink it to node_modules. I use link-package. You have to update babel to not ignore the symlinked folders.

PROJECT_ROOT/package.json

{
  "scripts": {
    "postinstall": "link-package ./app app"
  },
  //  ...
}

PROJECT_ROOT/app/package.json

{
  "name": "app",
  "standard": {
    "parser": "babel-eslint"
  }
}

PROJECT_ROOT/app/views/Bar/Bar.js

import { Foo } from 'app/components/Foo'
// ...

If you're in a larger project, you can instead symlink the folders inside app, since you're probably going to end up with lots of common shared components, libs, and utils.

PROJECT_ROOT/app/components/package.json

{
  "name": "components",
  "standard": {
    "parser": "babel-eslint"
  }
}

PROJECT_ROOT/app/views/Bar/Bar.js

import { Foo } from 'components/Foo'
// ...

This is what we use with our reasonably sized react app at Treasure Data. I think it works well. I've read over some of the weirder corner cases, but haven't stumbled into any myself.

This doesn't lock you into anything new, if you're already relying heavily on npm. There's so many modules that it's hard to imagine node's resolution algorithm going away any time soon. Aside from that, it tends to make it easy to integrate with other node tools.

A typical component folder: Foo/Foo.js, Foo/Foo.test.js, Foo/Foo.css, Foo/index.js. I use the index.js to explicitly specify the module's public API. e.g. Foo.js gets broken up into small chunks (which is usually easier to test), and it exports a few functions, but they're not meant to be used outside! I like being able to wall off sections of the app in my head.

I like using app instead of the actual app's name because otherwise names get too long.

Another benefit of using this style of import is that it sometimes makes refactoring stuff a little easier.

@cecilemuller
Copy link

@ccorcos I am not familiar with Node's own sources, but from a quick search, I think this is where: https://github.com/nodejs/node/blob/master/lib/module.js

@mlrawlings
Copy link

@ccorcos if I'm not mistaken, webpack uses its own module resolution that is separate from node's require.resolve https://github.com/webpack/enhanced-resolve

@ccorcos
Copy link
Author

ccorcos commented Sep 14, 2016

Resolve Project Root PR: #651

@amandapouget
Copy link

amandapouget commented Sep 21, 2016

Recognize the discussion above gets into this with more complexity, but for anyone stumbling across this and looking for tl;dr:

Absolute imports (of things in the src folder) can be accomplished with this simple modification to your package.json:

    "start": "NODE_PATH=src/ react-scripts start",
    "build": "NODE_PATH=src/ react-scripts build",
    "test": "NODE_PATH=src/ react-scripts test --env=jsdom",

Example: import { Banana } from 'fruits/yellow/big/Banana' where fruits is a subdirectory of src.

@gaearon
Copy link
Contributor

gaearon commented Sep 21, 2016

Thanks for the summary! I also think Windows Cmd users would need to write it as NODE_PATH=src&&react-scripts start etc because of different environment variable syntax. Alternatively you could install cross-env package that handles this in a cross-platform way and write cross-env NODE_PATH=src react-scripts start.

@ccorcos
Copy link
Author

ccorcos commented Sep 21, 2016

@mandysimon88 but suppose you want to install this package as an npm package and import a specific file -- the paths wont resolve because you need to build a custom build system. on the other hand, if you're resolving to the package name, then everything resolves just fine :)

For example, this works just fine:

mkdir cool-app
cd cool-app
npm init -y
echo "module.exports = { name: 'chet' }" > defs.js
echo "console.log('hello' + require('cool-app/defs').name)" > index.js
npm link
cd ..
mkdir
cd other-app
npm init -y
npm install --save cool-app
echo "require('cool-app')" > index.js
node index.js
> hello chet

And you dont need any special resolver / build system to get it to work because cool-app will resolve within other-app/node_modules. but if you're inside cool-app, it wont work :(

@jameslnewell
Copy link
Contributor

jameslnewell commented Sep 22, 2016

I'm not using CRA but I am following many of the decisions/conventions made here closely.

I wanted shortened paths for my apps but I didn't want to use my-app/module or src/module because those options have the possibility of clashing with existing npm packages and encourage the developer to shorten their package.json name - which may already be used by other tools outside of CRA.

Instead I'm using ~module.

e.g. ~actions/sendForm from src/components/ContactPage/ContactPage.jsx resolves to src/actions/sendForm.

Detailed thoughts behind ~module

Why use short paths?

In deeply nested directory structures, using relative paths to require modules above the current module directory can be tricky:

  • you have to know the right number of ..s to use
  • if you ever move the current module you'll need to update the number of ..s

Why use ~module?

I wanted a short prefix to save typing.

That rules out <package-name>/module because forcing developers to change their package name in order to achieve a shorter path is lame, and package names may already be used for other purposes.

I didn't want it to clash with the possible namespace of npm packages.

That rules out app/module, src/module, @app/module, @src/module etc because app, src, @app/module and @src/module are all valid npm package names.

I didn't want it to clash with existing operating system conventions (so it can be reused when bundling applications for NodeJS).

That rules out ~/ and /.

Why not use resolve.aliases or resolve.modules?

Using resolve.alias requires manual setup for every directory in your rootPath directory and results in confusion when resolve.aliases have been setup for some directories but not all of them.

Using resolve.module clashes with the possible namespace of npm packages and results in confusion over whether the imported module is a npm package in node_modules or is a local module in your src directory.

I've created a resolve plugin implementing the convention for Webpack v2 but I'm happy to implement a version for Webpack v1 and create a PR for this project using it if you agree with the approach. @gaearon

@amandapouget
Copy link

That’s fine with me if someone has a better approach. Mine is a “stop-gap."

On Sep 22, 2016, at 9:06 AM, James Newell notifications@github.com wrote:

I'm not using CRA but I am following many of the decisions/conventions made here closely.

I wanted shortened paths for my apps but I didn't want to use my-app/module or src/module because those options have the possibility of clashing with existing npm packages and encourage the developer to shorten their package.json name - which may already be used by other tools outside of CRA.

Detailed thoughts behind ~module

I've created a resolve plugin https://www.npmjs.com/package/webpack-resolve-short-path-plugin implementing the convention for Webpack v2 but I'm happy to implement a version for Webpack v1 and create a PR for this project using it if you agree with the approach. @gaearon https://github.com/gaearon

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #636 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AMQpjIyBB8SH9vfY5JCGJFGvKKKQS6VNks5qsn1TgaJpZM4J7IMK.

@gaearon
Copy link
Contributor

gaearon commented Sep 24, 2016

Proposal for absolute imports: #741

@gaearon
Copy link
Contributor

gaearon commented Sep 30, 2016

Going to close this, as it’s best to discuss in separate issues.
The absolute imports discussion will continue in #741.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests