Added production ready settings for react theme

[thangngoc89]

Fixes #2579

[chenglou]

Thanks! I'll try it.

[glennsl]

Using es6 by default is probably a bad idea, as it'll decrease compatibility with various tools. I think you've already had issues using es6 modules with bs-jest? commonjs on the other hand is the lowest-common denominator. As far I understand, the primary benefit of using es6 is to be able to show off tiny hello world bundles, but I don't think that is justification enough for increasing friction for newcomers.

[chenglou]

@thangngoc89 what do you think?

[thangngoc89]

@glennsl I've successfully config bs-jest for working with es6, this is my current config:

"jest": {
    "transformIgnorePatterns": [
      "/node_modules\\/(?!bs-jest|bs-platform|reason-react|bs-react-test-renderer)/"
    ],
    "testMatch": [
      "**/__tests__/**/*_test.bs.js"
    ]
  }

This is nothing crazy, I'm telling jest to transpile bs-jest, bs-platform, reason-react, bs-react-test-renderer and friends because jest doesn't transpile anything in node_modules by default. Perhaps you would accept a PR to bs-jest's README about this @glennsl ?

the primary benefit of using es6 is to be able to show off tiny hello world bundles

Hmm, the main benefit is definitely not for the hello world, It's a good default for treeshaking and a small bundle in real world app. The ecosystem has adapted with ES6 modules and I would say it's sane to enable it right now.

[thangngoc89]

A sidenote: You could set transformIgnorePatterns to [ ""] but that would introduced a lot of overhead.

[chenglou]

Are we sure that es6 also works well with other tools than bs-jest?

[yawaramin]

Let me throw in my 2c. ES6 is the standard in frontend dev. If we want to be taken seriously as a replacement for Babel and friends, we need to show we care about bundle size.

[glennsl]

Perhaps you would accept a PR to bs-jest's README about this @glennsl ?

Definitely, but I don't think this is actually all the required configuration. Don't you need to add babel-jest too?

Hmm, the main benefit is definitely not for the hello world, It's a good default for treeshaking and a small bundle in real world app. The ecosystem has adapted with ES6 modules and I would say it's sane to enable it right now.

The few who require tiny bundle sizes can probably figure out how to enable es6 modules through some guide. I don't think it's a good idea to (possibly) sacrifice newcomer experience for the very minor convenience of having an advanced optimization enabled by default. At the very least I'd like to see this tested for a bit to have a better idea of its consequences before enabling it as the default.

[yawaramin]

Makes sense. We can test it in master and release it (or not) based on that.

[thangngoc89]

Definitely, but I don't think this is actually all the required configuration. Don't you need to add babel-jest too?

Yep. I do. I think it's best that we change this back to commonjs. Also I want to see this happens #2418 . This won't require a custom config for jest

[chenglou]

Ok, let's use commonjs and put a comment right above on using es6 (bsconfig.json parser accepts comments and trailing commas).

Edit: or maybe put it in webpack.config.js or README? @thangngoc89?