switch back to official standard-minifier-js #3363

jshimko · 2017-12-01T14:55:26Z

This resolves #2629 where the production version of React wasn't being used in production builds. We already made this switch when updating to Meteor 1.5 (#2371), but I don't know why we switched back to abernix:standard-minifier-js shortly after that. Somewhere between June 7 (where Meteor 1.5 was merged) and the release of Reaction 1.3.0 on June 21 it appears that it got reverted to abernix:standard-minifier-js. Possibly a mistake during a merge conflict resolution? There are no commits to the .meteor/packages file during that period that directly reference changing that package, so I'm thinking it had to be unintentional during a merge conflict.

Anyway, please give this a thorough test and see if you can find any build issues. I've done a full production build/deployment with Docker and haven't seen any issues.

aaronjudd

Testing builds:

No errors seen in development mode.

Testing production build with fresh clone, ran:

meteor build --directory /tmp/
cd /tmp/bundle/programs/server
npm install
cd ../../
ROOT_URL="http://localhost:3000" MONGO_URL="mongodb://localhost:27017/reaction" PORT="3000" node main.js

No issues seen in production build.

Tested react:

function isDevReact() {
  try {
    React.createClass({});
  } catch(e) {
    if (e.message.indexOf('render') >= 0) {
      return true;  // A nice, specific error message
    } else {
      return false;  // A generic error message
    }
  }
  return false;  // should never happen, but play it safe.
}; 
isDevReact();

and also

if (!process.env.NODE_ENV || process.env.NODE_ENV === 'development') {
    console.log("development");
} else {
    console.log("production");
}

Both the swag.getreaction.io (Reaction / Meteor 1.6.0 without the standard minifier) and hello.reactioncommerce.com return "not development" mode as well.

Is there another way to verify if this is resolving #2629 or #1896? I see the same results (production) on all cases...

I was not able to build a docker image locally using:

 docker build --build-arg TOOL_NODE_FLAGS="--max-old-space-size=4096" -t reactioncommerce/reaction:latest .

Hangs on

[-] Building Meteor application...

Can we clarify acceptance criteria?

brent-hoover · 2017-12-02T01:35:36Z

I've been using the React developer tool plugin to verify. swag.getreaction.io still gives me

This page is using the development build of React. 🚧

Note that the development build is not suitable for production. 
Make sure to use the production build before deployment.

I guess we need to dig into the plugin a little and determine how exactly it's deciding this is the development build

brent-hoover · 2017-12-02T01:43:08Z

A quick check of the code says

  function detectReactBuildType(renderer) {
    try {
      if (typeof renderer.version === 'string') {
        // React DOM Fiber (16+)
        if (renderer.bundleType > 0) {
          // This is not a production build.
          // We are currently only using 0 (PROD) and 1 (DEV)
          // but might add 2 (PROFILE) in the future.
          return 'development';

I haven't figured out how to get that renderer object yet,

brent-hoover · 2017-12-02T11:49:18Z

Found these. Looks like this switch was made to fix an issue with Docker builds?

#2475
#2486
^^ that pull on the 22nd of June seems to be where the abernix fork was reintroduced?

aaronjudd

Was able to verify react now in production, and after clearing all local images, etc. was able to build a reaction image locally. Approved.

aaronjudd · 2017-12-04T18:22:51Z

After not seeing any error, I thought the build had succeeded, but the image wasn't actually created, and after re-running twice more, both the docker build attempts failed. Are there other recommended config options?

docker build --build-arg TOOL_NODE_FLAGS="--max-old-space-size=2048" -t reactioncommerce/reaction:latest .

/opt/build_scripts/build-meteor.sh: line 30:  5174 Killed                  meteor build --directory $APP_BUNDLE_DIR
The command '/bin/sh -c cd $APP_SOURCE_DIR &&             $BUILD_SCRIPTS_DIR/install-deps.sh &&             $BUILD_SCRIPTS_DIR/install-node.sh &&             $BUILD_SCRIPTS_DIR/install-phantom.sh &&             $BUILD_SCRIPTS_DIR/install-mongo.sh &&             $BUILD_SCRIPTS_DIR/install-meteor.sh &&             $BUILD_SCRIPTS_DIR/build-meteor.sh &&             $BUILD_SCRIPTS_DIR/post-build-cleanup.sh' returned a non-zero code: 137```

Unable to build

aaronjudd

On OS X - I've now completed multiple tests.

Successfully built master

Removing intermediate container 4dfbc56d30ca
Successfully built b8cc37047361
Successfully tagged reactioncommerce/reaction:latest

Repeated attempts to build this branch, all returned a non-zero code: 137, and didn't create/tag the new images.

jshimko · 2017-12-07T18:15:51Z

This just worked for me multiple times on both Mac and Linux.

docker build --build-arg TOOL_NODE_FLAGS="--max-old-space-size=2048" -t reactioncommerce/reaction:latest .

So am I the only person not running into the issue?

spencern · 2017-12-07T18:34:40Z

pulling a fresh copy and reviewing now.

spencern

I was able to build successfully
Steps:

reaction init new-directory
git checkout standard-minifier-update
reaction
ctrl-c
docker build --build-arg TOOL_NODE_FLAGS="--max-old-space-size=2048" -t reactioncommerce/reaction:latest .

Building dependency tree...
Reading state information...
 ---> fbd541991807
Removing intermediate container afcacf82fcf2
Removing intermediate container 8f675b6a7f6a
Removing intermediate container f63777deda1e
Removing intermediate container dd7d19e0da35
Removing intermediate container b3572946c52a
Removing intermediate container 0e52f48728c1
Removing intermediate container 21203aede6fe
Removing intermediate container 00a59ec218c4
Removing intermediate container dd90187972b0
Removing intermediate container 72ea578fce83
Removing intermediate container 4267a425feb2
Removing intermediate container 51c9b31371e2
Step 2/3 : ENV ROOT_URL "http://localhost"
 ---> Running in afff691b1351
 ---> b0a1908b7efe
Removing intermediate container afff691b1351
Step 3/3 : ENV MONGO_URL "mongodb://127.0.0.1:27017/reaction"
 ---> Running in 7db59774d658
 ---> c2dfdc6f1c93
Removing intermediate container 7db59774d658
Successfully built c2dfdc6f1c93
Successfully tagged reactioncommerce/reaction:latest

BitHound reporting two failures, but also has a blank screen.

eslint . in the project directory returns no errors. Coadacy returns no errors.

Any idea what BitHound is concerned about or is this just the latest reason to leave BH?

brent-hoover · 2017-12-08T01:39:41Z

I followed the steps that Spencer outlined above and it failed. I then switched to master and it worked.

Also, Bithound be gone.

- resolve issue with Mongo 3.4+ connectivity

Successfully building locally

aaronjudd

Success with:

docker build --build-arg TOOL_NODE_FLAGS="--max-old-space-size=2048" -t reactioncommerce/reaction:latest .
docker-compose -f docker-compose.yml up

switch back to official standard-minifier-js

switch to official standard-minifier-js

4c11902

jshimko requested review from aaronjudd and spencern December 1, 2017 14:55

aaronjudd reviewed Dec 1, 2017

View reviewed changes

aaronjudd previously approved these changes Dec 4, 2017

View reviewed changes

aaronjudd previously requested changes Dec 4, 2017

View reviewed changes

spencern reviewed Dec 7, 2017

View reviewed changes

aaronjudd added 2 commits December 8, 2017 12:15

Add reaction-connectors-shopify package-lock

f80b674

Add --bind_ip_all to mongod

64dbf62

- resolve issue with Mongo 3.4+ connectivity

aaronjudd approved these changes Dec 8, 2017

View reviewed changes

aaronjudd mentioned this pull request Dec 8, 2017

(fix) 3360 [email protected] #3385

Merged

spencern changed the base branch from master to release-1.6.1 December 8, 2017 22:41

spencern merged commit 8cd2272 into release-1.6.1 Dec 8, 2017

spencern deleted the standard-minifier-update branch December 8, 2017 22:48

spencern mentioned this pull request Dec 12, 2017

Release 1.6.1 #3394

Merged

Akarshit pushed a commit that referenced this pull request Jan 7, 2018

Merge pull request #3363 from reactioncommerce/standard-minifier-update

a82ad98

switch back to official standard-minifier-js

brent-hoover mentioned this pull request Jan 23, 2018

Development version of React in production #2629

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

switch back to official standard-minifier-js #3363

switch back to official standard-minifier-js #3363

jshimko commented Dec 1, 2017

aaronjudd left a comment •

edited

Loading

brent-hoover commented Dec 2, 2017

brent-hoover commented Dec 2, 2017 •

edited

Loading

brent-hoover commented Dec 2, 2017

aaronjudd left a comment

aaronjudd commented Dec 4, 2017 •

edited

Loading

aaronjudd left a comment

jshimko commented Dec 7, 2017

spencern commented Dec 7, 2017

spencern left a comment

brent-hoover commented Dec 8, 2017

aaronjudd left a comment

switch back to official standard-minifier-js #3363

switch back to official standard-minifier-js #3363

Conversation

jshimko commented Dec 1, 2017

aaronjudd left a comment • edited Loading

Choose a reason for hiding this comment

brent-hoover commented Dec 2, 2017

brent-hoover commented Dec 2, 2017 • edited Loading

brent-hoover commented Dec 2, 2017

aaronjudd left a comment

Choose a reason for hiding this comment

aaronjudd commented Dec 4, 2017 • edited Loading

aaronjudd left a comment

Choose a reason for hiding this comment

jshimko commented Dec 7, 2017

spencern commented Dec 7, 2017

spencern left a comment

Choose a reason for hiding this comment

brent-hoover commented Dec 8, 2017

aaronjudd left a comment

Choose a reason for hiding this comment

aaronjudd left a comment •

edited

Loading

brent-hoover commented Dec 2, 2017 •

edited

Loading

aaronjudd commented Dec 4, 2017 •

edited

Loading