Upgraded to Babel 7

[thakkaryash94]

closes #499 . Upgraded tested dev and prod build.

[codecov-io]

Codecov Report

Merging #501 into master will not change coverage. The diff coverage is 100%.

@@          Coverage Diff           @@
##           master    #501   +/-   ##
======================================
  Coverage    84.1%   84.1%           
======================================
  Files         171     171           
  Lines        5387    5387           
  Branches        1       1           
======================================
  Hits         4531    4531           
  Misses        856     856

Impacted Files	Coverage Δ
src/bootstrap/index.js	75% <ø> (ø)	:arrow_up:
src/scintillator/nodes/object.js	97.43% <100%> (ø)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f822a33...5b0cd82. Read the comment docs.

[thakkaryash94]

netlify build is live at https://deploy-preview-501--bemuse.netlify.com/

[thakkaryash94]

Error is08 10 2018 06:03:30.410:WARN [HeadlessChrome 0.0.0 (Linux 0.0.0)]: Disconnected (1 times). We can try rerun it and check if it's the same.

[resir014]

@thakkaryash94 yep, just reran on CircleCI, still the same

[thakkaryash94]

I reset HEAD to old commit remove postinstall scriptand ran test locally, they are failing as well.

[resir014]

Hmm, that's odd.

I have a currently-active PR to upgrade all the dependencies outside of Babel. Would you like me to get that done and merged first so you could rebase off of it?

[thakkaryash94]

Sure, we can try. I locally merge deps-upgrade-v42 branch and run test, it failed too. Let's see.

[resir014]

@thakkaryash94 Have you tried running the in-browser tests and see if they work?

http://<localhost>/?mode=test

[thakkaryash94]

tested locally, only below 4 test cases are failing.

https://github.com/bemusic/bemuse/blob/master/src/scintillator/index.spec.js#L245 https://github.com/bemusic/bemuse/blob/master/src/scintillator/index.spec.js#L247

https://github.com/bemusic/bemuse/blob/master/src/scintillator/index.spec.js#L274 https://github.com/bemusic/bemuse/blob/master/src/scintillator/index.spec.js#L276

[resir014]

Ah, Scintillator. 😅 I knew it's gonna be a problem at some point...

I'll ask our other maintainer @dtinth to have a look.

[dtinth]

Hi, sorry for not following on this PR. I was busy with lots of stuff. I am digging down to find the root cause of the problem.

The test case causes Chrome Renderer Process to crash with this message logged into the console:

<--- Last few GCs --->
me[60603:0x7fba9e848200]     1953 ms: Mark-sweep 262.2 (303.6) -> 193.9 (238.9) MB, 10.7 / 0.0 ms  (+ 2.7 ms in 1 steps since start of marking, biggest step 2.7 ms, walltime since start of marking 144 ms) (average mu = 0.950, current mu = 0.950) allocation [60603:0x7fba9e848200]     3746 ms: Mark-sweep 1688.6 (1733.7) -> 990.7 (1035.7) MB, 39.7 / 0.0 ms  (+ 1.8 ms in 1 steps since start of marking, biggest step 1.8 ms, walltime since start of marking 1128 ms) (average mu = 0.974, current mu = 0.977) allocat

<--- JS stacktrace --->

==== JS stack trace =========================================

    0: ExitFrame [pc: 0x1098bba2e]
Security context: 0x004ab9c4aa41 <String[21]: http://localhost:8080>
    1: onChildrenChange [0x4a6176ac49] [0x004afb5826f1 <undefined>:~255] [pc=0x40ea91a77b](this=0x004a81174241 <ParticleContainer map = 0x4ae8393769>,1)
    2: addChild [0x4aa92d7639] [0x004afb5826f1 <undefined>:102] [bytecode=0x4a70efeb19 offset=146](this=0x004a81174241 <ParticleContainer map = 0x4ae8393769>,0x004a8115b9c9 <Sprite map ...

This is really bizarre... I am figuring out why right now.

[dtinth]

I tried to step through the code line-by-line, and I reached a point where it crashed.

The crash

The crash lies in this function, inside node_modules/pixi.js/lib/particles/ParticleContainer.js.

    ParticleContainer.prototype.onChildrenChange = function onChildrenChange(smallestChildIndex) {
        var bufferIndex = Math.floor(smallestChildIndex / this._batchSize);

        while (this._bufferUpdateIDs.length < bufferIndex) {
            this._bufferUpdateIDs.push(0);
        }
        this._bufferUpdateIDs[bufferIndex] = ++this._updateID;
    };

When I tried to log, bufferIndex is NaN, causing an infinite while loop where this._bufferUpdateIDs is getting pushed with 0 until Chrome crashes due to out-of-memory error.

The reason of crash

I look at what could have caused bufferIndex to become NaN, and I found out that...

1. We created a ParticleContainer with maxSize of null. This should trigger the default of 1500

   https://github.com/bemusic/bemuse/blob/f822a33371c2d97e12d1ef3af16db272b6444be4/src/scintillator/nodes/object.js#L77-L80

2. However, the commit https://github.com/pixijs/pixi.js/commit/22c5248a51059979278b7cefc11e3a7e7078c008 that gets released in Pixi v4.5.0 contained this change:

   -class ParticleContainer extends core.Container {
   +export default class ParticleContainer extends core.Container
   -
   -    constructor(maxSize, properties, batchSize)
   +    constructor(maxSize = 1500, properties, batchSize = 16384)
       {
           super();
   
   -        batchSize = batchSize || 15000; //CONST.SPRITE_BATCH_SIZE; // 2000 is a nice balance between mobile / desktop
   -        maxSize = maxSize || 15000;
   

   Instead of using || to default maxSize to 1500 when it is falsy, it has been changed to use ES6 default parameters which only works with undefined. This means null value will no longer set maxSize to a default of 1500. This caused bufferIndex to subsequently become NaN, causing an infinite loop.

3. Our yarn.lock in master pins Pixi.js to v4.1.0:

   pixi.js@^4.1.0:
    version "4.1.0"

   but in this PR, it has been upgraded to v4.8.2, which contains the change that caused the crash:

   pixi.js@^4.1.0:
    version "4.8.2"

   Has the yarn.lock been removed and entirely rebuilt from scratch in the process?

How to fix the crash

Change null to undefined.

    let batch = new PIXI.particles.ParticleContainer(undefined, {
      position: true,
      alpha: true
    })

[dtinth]

I pushed a fix to your branch — 075cc4b.

[dtinth]

Pushed another fix that makes the lint job pass now — 5b0cd82

[thakkaryash94]

I was adding and removing babel deps when migrating, so removed yarn.lock completely and regenerated, my mistake. I think it will better if start using fix package versions instead of^(caret). So that something like this won't happen in future.

[dtinth]

@thakkaryash94 I agree that could prevent the issue in the future. However I think it’s quite strange as non-major version bumps shouldn’t cause the app to crash, and upgrading Babel shouldn’t force Pixi to bump version.

Another possible fix is to revert yarn.lock to master version, and run yarn install again. This should keep the dependencies pinned as much as possible.

I verified this by running:

git checkout master -- yarn.lock
yarn install

And looking at the yarn.lock file, and yes, Pixi.js stays at v4.1.0. So that’s another possible solution. Personally I don’t want to pin the dependencies in package.json because I expect our app to be able to use with latest compatible dependency version, and if it doesn’t, our tests should catch it.

But I think, for this PR, there is no need to revert yarn.lock anymore, since we’ve fixed the compatibility issue. Clearing yarn.lock and fixing issues from time to time is also a good thing!