react-webpack-generators / react-webpack-template Goto Github PK

As brought up in generator-react-webpack #266

V1

• remove open() method in server.js
• log dev server address to console after the inital webpack build is ready

This workaround just sets a Webpack alias so that running the stable line of the template / generator (v1, v3) don't present users with a red-screen. The long-term solution is discussed here:

• #72
• react-webpack-generators/generator-react-webpack#302

@weblogixx Hey there!

Try restarting the build. A week ago it started to fail for me with "PhantomJS crashed", I had to roll back to an older setup (karma-phantomjs-launcher@^0.2.1, phantomjs instead of phantomjs-prebuilt, etc...). Reverting this basically.

Just wanted to give you a heads up and also maybe you'll have a better idea what causes this.
Note: locally everything works fine. Just travis fails

@weblogixx I remember talking about HMR/hot-reloading briefly. Did you consider or have any experience with using v3 (currently in beta) with the template? I am starting a test on one project now but was curious because I remember that you were talking about some HMR issues while working on the new repos.

Check out https://github.com/airbnb/enzyme for better testing of components.

As stated in react-webpack-generators/generator-react-webpack#243, components should be createable without own styles via yo react-webpack:component my/namespaced/components/name --nostyle.

E.g. when running the tests

The react/require-extension rule is deprecated. Please use the import/extensions rule from eslint-plugin-import instead.

Update Blocked by issue 978 on airbnb/javascript
I've subscribed to the issue, once it is ready we will probably only have to update the dependency to the airbnb rules.

Currently we set the NODE_ENV by mapping 1:1 to our Webpack configs. When working with multiple environments (e.g. dev, staging, dist) I found the requirement for finer control. E.g. to achieve this

dev     -> NODE_ENV=dev,        APP_ENV=dev
staging -> NODE_ENV=production, APP_ENV=staging
dist    -> NODE_ENV=production, APP_ENV=dist

In this example, staging and dist should both build with production optimizations (e.g. webpack -p and NODE_ENV=production). However, staging may differ in other conditional flags used throughout the app. This means, inside the project I need to distinguish between code targeted for staging and production, which I do by means of the APP_ENV setting.

Initially I was using the setting from src/config/<env>.js to pick appropriate values. However, in the case of dynamic requires Webpack would not eliminate dead code that way, so I'd propose using the DefinePlugin and introduce two changes to the configs:

// Somewhere inside conf/webpack/Staging.js
get env() {
  return process.env.NODE_ENV || 'production';
}

get appEnv() {
  return process.env.APP_ENV || 'staging';
}

this.config = {
  plugins: [
    new webpack.DefinePlugin({
      'process.env': {
        NODE_ENV: JSON.stringify(this.env),
        APP_ENV: JSON.stringify(this.appEnv)
      }
    }),
    // ...
  ]
};

This way, process.env.APP_ENV becomes available in code and everything inside the branch will be eliminated from the bundle size when the condition is not met (I am using it for things like requiring gobal mocks, adding dev routes, dev components, etc.).

@weblogixx would this addition be okay for you?

Hey @weblogixx,
as you know I am right now working on a Redux generator on top of generator-react-webpack, which uses this repo as base.

Since Redux handles things slightly different I came across something:

I think it would be better to move run.js out of the components folder and into the src folder directly, for the following reasons:

• I do not think, that this really counts as a component (since it does not extend React.component)
• Redux has components and containers, containers being smart components, so basically the top component in a Redux context is a container.
  This means that in my Redux generator I have to parse the configs and change the path of the run script, add my run script and the main container, delete the run script from the components dir. So if the run.js is moved to the root of src, I would only need to rewrite run.js without the need of parsing any files.

TLDR: I would like to move /src/components/run.js to /src/index.js and adapt the config accordingly. If you too think that this makes sense, let me know and I will submit a PR.

I have just realized that the template's App.js and app.cssmodule.css implicitly assume a generator setup that will use css modules. When installing a V4 setup with prompting no for cssmodules, the dependency for "react-css-modules": "^3.7.6" is still present in package.json (thus it gets installed no matter what the user answers the prompt, and App.js will always have the @cssmodules annotation).

I see four paths from here:

1. pulling out App.js/app.cssmodule.css from the template and into the generator
2. defaulting App.js and app.css not to use css modules
3. stop associating app.js with a style file at all
4. keeping App.js and app.cssmodule.css as default in the template project and overriding it from the generator if necessary

I'd argument that 1 is the worst of these options, since the template would no longer work standalone, which I'd consider not being an option.

2, 3, and 4 should all be possible. 4 would require custom code (involving the deletion of app.cssmodule.css) and feel a bit unnatural, so at least as of now I think that I am pro 2 or 3.

@weblogixx what's your opinion, do I miss something more obvious?

Here is a suggestion for an improvement to webpack config. Replace resolve.alias with resolve.root.

Why? Alias is bad because it applied to all imports, and in the current state is adds an extra slash breaks some imports.

E.g. importing 'helpers/actions/ActionWrapper.js' would convert to 'helpers/actions//ActionWrapper.js'

Or am I missing something about the reason it was used?

Instead, add to the search path using resolve.root.

PR submitted.

The entry in Base.js is overwritten from either dev, dist or test from what I can see. As far as I understand it points to the wrong entry file, but since it is never used, I guess it does not matter.

Should this be removed? Or at least adjusted to client.js?

Reported here => stylesuxx/generator-react-webpack-redux#56

Originates from: react-webpack-generators/generator-react-webpack#191

As discussed in react-webpack-generators/generator-react-webpack#158

Currently we have a static index.html as entry point, which imposes a few limitations on more advanced use cases like server-side-rendering or modifying page parameters like asset paths (generator:#233), meta tags, or cache-hashes (generator:#217) at build-time.

To work around that, most other kits that I know work with some sort of templating language (template strings, EJS, React) to build the index.html dynamically. During that build we have full access to the bundle, including Webpack's stats object to access filenames, hashes, & co.

This would be a bigger update, but if you are interested in what it might look like I could setup a demo branch to discuss the architecture (and after that we could decide whether to follow that path or discard it).

The port for the dev server reflects in two places:

• Base.js#L81
• Dev.js#L16

This value should be populated from a single setting and additionally be overridable from outside through an env var (e.g. for spinning up a dev server on a separate port wo/ needing to modify the config file that is under VC).

Hi,

I was wondering if there is an easy way to add express when running npm run dist? I keep trying without any luck.

Please and thank you,

Mike

Webpack's DefinePlugin can help reducing output file size by letting the uglifier remove code blocks that are only relevant for non-production builds.

By adding it like this into the config.plugins in cfg/dist.js the app.js output size drops from 192K to 135K.

plugins: [
  new webpack.optimize.DedupePlugin(),
  new webpack.DefinePlugin({
    'process.env': {
      'NODE_ENV': '"production"'
    }
  }),
  new webpack.optimize.UglifyJsPlugin(),
  new webpack.optimize.OccurenceOrderPlugin(),
  new webpack.optimize.AggressiveMergingPlugin(),
  new webpack.NoErrorsPlugin()
]

The use is also mentioned in the React Docs

Note: by default, React will be in development mode. To use React in production mode, set the environment variable NODE_ENV to production (using envify or webpack's DefinePlugin). A minifier that performs dead-code elimination such as UglifyJS is recommended to completely remove the extra code present in development mode.

If that's interesting I could provide a PR.

Since isparta-loader is now a deprecated package (together with isparta-webpack-loader, which we used before...), we should update coverage to use https://github.com/istanbuljs/babel-plugin-istanbul.

@weblogixx could we release a new beta build for the template's 2.x branch, so that the updates to the loader configuration for style files (with/without css modules for all css preprocessors) become available in the V4 branch of the generator?

As referenced here, I found the hot update returning 404's with the leading period in the output.publicPath property defined in cfg/base.js when using the History API (specifically, React Router). I don't think this problem is unique to me because I didn't modify the config files.

When creating --stateless components the template creates:

const Footer = () => {
  return (
    // jsx here
  );
};

With the current eslint settings, this won't compile:

3:22 error Unexpected block statement surrounding arrow body arrow-body-style

In terms of the eslint rules, this is intended as stated here. The reason is, that the arrow body only consists of the return statement, thus eslint would only validate it as:

const Footer = () => (
  // jsx here
);

Obviously this isn't hard to fix, either by altering the eslint config, adding some variable into the body or refactoring the arrow to return with (). The more important question is, what's better for users of the generator, will they be more likely to extend the body or use it as a plain return stmt.

I am against weakening the eslint rules, and IMO both use cases are equally important, so maybe another option to the generator makes sense. Otherwise we should probably pick (whichever) default that validates out-of-the-box. As over-engineering is not a good habit, I am +1 for picking a default.

I will provide the PR based on a decision in this issue.

As noted in babel-preset-es2015-native-modules it should be updated to

presets: [
  ["es2015", { "modules": false }]
]

Currently the build on Node 4 / npm 3 fails due to unsatisfied peer deps. As tests pass with npm 3, we decided to update the npm requirement to 3+ and test whether the build passes again.

The general issue with unsatisfied peer deps is discussed in this issue react-webpack-generators/generator-react-webpack#252.

Sometimes it could be useful to change the ports of the dev or test servers, e.g. by starting them like

SERVER_PORT=<port_number> npm start
SERVER_PORT=<port_number> npm run serve:dist
TEST_SERVER_PORT=<port_number> npm test

I will refer a short commit to this issue. If you find the feature worthwhile it might be a way to implement it.

When running npm run serve:dist, the content is served from src directory instead of dist directory.

The cfg/dist.js has one of the entries webpack-dev-server/client?http://127.0.0.1:8000. 8000 should the port defined in cfg/defaults.js

As wanted in react-webpack-generators/generator-react-webpack#157, the webpack configuration should be able to read bower files.

Support for Babel 6 can be integrated for components without any bigger problems. However, unit testing is currently not working because of douglasduteil/isparta#81.

As described in this issue react-webpack-generators/generator-react-webpack#154 (comment) this feature would allow to import a configuration object based on the current --env by using import config from 'config';.

The config modules are loaded from:

src/config/dev.js
src/config/dist.js
src/config/test.js

HMR is telling me:

Consider using the NamedModulesPlugin for module names.

This can be achived by adding the new webpack.NamedModulesPlugin() to the Plugin section of webpack/Dev.js

Changes HMR log messeges from

[WDS] App hot update...
log-apply-result.js:20 [HMR] Updated modules:
log-apply-result.js:22 [HMR]  - 2
log-apply-result.js:22 [HMR]  - 69

to:

[WDS] App hot update...
log-apply-result.js:20 [HMR] Updated modules:
log-apply-result.js:22 [HMR]  - ./components/App.js
log-apply-result.js:22 [HMR]  - ./containers/App.js

If this is something we want, feel free to assign me and I will make the change.

In #54 we've updated the build step to support copying a static directory to dist. With #49 we are working on solutions that support running and building on non-root server directories. This requires the static files directory to be copied over to a possibly deeper location than /static/.

To achieve this, we need access to Webpack's stats object, which we don't get easily via a build step in the copy run script. Refactoring the copy operation to copy-webpack-plugin will resolve that and integrate it nicely into the Webpack workflow.

@weblogixx do you have any objections to this? Otherwise I would go ahead integrating it.

Lately I've started hitting the below error on two build machines (Node 6.2.x and 6.3.x) and it was also reported here stylesuxx/generator-react-webpack-redux#45.

Module build failed: Error: Couldn't find preset "es2015" relative to directory "."

We might have to check whether something changed in the type of dependencies that lets babel-preset-es2015 miss all the sudden. If so we can add it to the devDependencies.

As brought up in generator-react-webpack #266

V2

• remove the --open call on npm start
• log dev server address to console after the inital webpack build is ready

@weblogixx should I tackle this?

Possibly an error related to configuration of HMR.

bootstrap 5db23fd…:19Uncaught Error: [HMR] Hot Module Replacement is disabled.

What is your opinion on splitting the root test directory into

test/
  func/
    ... everything that is currently under test/
  unit/
    ... root directory for unit tests run by mocha

Currently I've added a mocha run script to package.json.

"mocha": "./node_modules/mocha/bin/mocha --require babel-core/register"

and run some unit tests by:

npm run mocha -- test/fixtures/qaFixturesTest.js --watch

That's a quite manual way and it would probably be nicer api for our users if there were run scripts like npm run test:unit, npm run test:func. npm run test would run both of them.

Originates from: react-webpack-generators/generator-react-webpack#270

• add directory src/static
• provide README in src/static
• update copy run script to recursively copy the directory to dist/static
• refactor favicon to be copied via the static dir
• reference favicon from new location in index.html

Note: in further iterations the build step could be improved by #49, which will allow us to build dynamic/configurable paths to the static/asset/... directories (e.g. when users build apps to be hosted on sub-directories, where src="/static/foo.jpg" would fail).

I would propose renaming src/index.js for two reasons:

1. tooling
2. extendability

1. Various editors/ides will support some kind of file opening dialog. Due to index.js being npm's default name for packages and often being used as an entry point for package exports, looking only for index.js in a regular project brings up many files.

2. To extend the setup, e.g. for server-side-rendering, we often create a second entry point for the server build. A naming pair of server.js and client.js (or whatever) is more self-documenting than e.g. index.js and client.js.

Moved from react-webpack-generators/generator-react-webpack#248

After installing the 4.x generator with CSSModules + Preprocessor (e.g. LESS) and creating a new component the component template creates

- Foobar.js
- foobar.cssmodule.less

import styles from './foobar.cssmodule.less';
// ...
<div className="foobar-component" styleName="foobar-component" />

Rendering will fail with an exception of css module is undefined, because the loaders in cfg/base are configured for for the *.less files only and miss a test against *.cssmodule.less and a loader that is adapted for cssmodules ('css?modules&importLoaders=1&localIdentName=[name]-[local]-[hash:base64:5]').

WARNING in webpack: Using NoErrorsPlugin is deprecated.
Use NoEmitOnErrorsPlugin instead.

Easily fixed in webpack/Dev.js & webpack/Dist.js by changing last Plugin from NoErrorsPlugin to NoEmitOnErrorsPlugin.

Feel free to assign me to this.

Related to: react-webpack-generators/generator-react-webpack#275

Additions to test/.eslintrc

• env
  • mocha (mutes warnings for missing imports in certain IDEs)
• rules
  • prefer-arrow-callback: 0 (don't err out on inline func callbacks)
  • func-names: 0 (remove warning for unnamed inline funcs)
  • strict: 0 (older node versions may need use strict for certain keywords, e.g. let, const)

Update tests

Update existing tests to use inline functions instead of arrow functions (recommended by official Mocha docs)