Skip to content

Conversation

mreishus
Copy link
Contributor

Creating a query builder, loading it with data, then destroying it was
causing some memory to be retained in the browser.

This should fix.

This fix was created by my colleague w/ @claudior2184 .

Merge request checklist

  • I read the guidelines for contributing
  • I created my branch from dev and I am issuing the PR to dev
  • I didn't pushed the dist directory
  • Unit tests are OK
  • If it's a new feature, I added the necessary unit tests
  • If it's a new language, I filled the __locale and __author fields

Creating a query builder, loading it with data, then destroying it was
causing some memory to be retained in the browser.

This should fix.

This fix was created by my colleague w/ @claudior2184 .
@mistic100
Copy link
Owner

I am not against this change but are you sure it fix anything ? I don't see any reason for these properties not be GC.

@mreishus
Copy link
Contributor Author

Use this test case, save as HTML and drag to browser.

<!-- Imports -->
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/css/bootstrap.min.css" integrity="sha384-1q8mTJOASx8j1Au+a5WDVnPi2lkFfwwEAa8hDDdjZlpLegxhjVME1fgjWPGmkzs7" crossorigin="anonymous">

<script src="https://code.jquery.com/jquery-2.2.3.min.js"></script>

<script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/js/bootstrap.min.js" integrity="sha384-0mSbJDEHialfmuBBQP6A4Qrprq5OVfW37PRR3j5ELqxss1yVqOtnepnHVP9aJ7xS" crossorigin="anonymous"></script>

<script src="http://cdn.jsdelivr.net/npm/jQuery-QueryBuilder/dist/js/query-builder.standalone.min.js">
</script>

<!-- Application -->
<div>
  <button id="1kbtn">Make 1k QB</button>
  
  <div id="builder-basic" />
  
</div>

<script>
function createQB() {
  var rules_basic = {
    condition: 'AND',
    rules: [{
      id: 'price',
      operator: 'less',
      value: 10.25
    }, {
      condition: 'OR',
      rules: [{
        id: 'category',
        operator: 'equal',
        value: 2
      }, {
        id: 'category',
        operator: 'equal',
        value: 1
      }]
    }]
  };
  var filters_basic = [
  {
      id: 'name',
      label: 'Name',
      type: 'string'
    }, {
      id: 'category',
      label: 'Category',
      type: 'integer',
      input: 'select',
      values: {
        1: 'Books',
        2: 'Movies',
        3: 'Music',
        4: 'Tools',
        5: 'Goodies',
        6: 'Clothes'
      },
      operators: ['equal', 'not_equal', 'in', 'not_in', 'is_null', 'is_not_null']
    }, {
      id: 'in_stock',
      label: 'In stock',
      type: 'integer',
      input: 'radio',
      values: {
        1: 'Yes',
        0: 'No'
      },
      operators: ['equal']
    }, {
      id: 'price',
      label: 'Price',
      type: 'double',
      validation: {
        min: 0,
        step: 0.01
      }
    }, {
      id: 'id',
      label: 'Identifier',
      type: 'string',
      placeholder: '____-____-____',
      operators: ['equal', 'not_equal'],
      validation: {
        format: /^.{4}-.{4}-.{4}$/
      }
    }
  ];

  $('#builder-basic').queryBuilder({
    plugins: ['bt-tooltip-errors'],
    filters: filters_basic,
    rules: rules_basic
  });

  $('#btn-reset').on('click', function() {
    $('#builder-basic').queryBuilder('reset');
  });

  $('#btn-set').on('click', function() {
    $('#builder-basic').queryBuilder('setRules', rules_basic);
  });

  $('#btn-get').on('click', function() {
    var result = $('#builder-basic').queryBuilder('getRules');

    if (!$.isEmptyObject(result)) {
      alert(JSON.stringify(result, null, 2));
    }
  });
  
}

function destroyQB() {
  $("#builder-basic").queryBuilder("destroy");
}

function make1kQB() {
  console.log('make1kqb begin');
  for (var i = 0; i <= 1000; i++) {
    createQB();
    //console.log(i);
    destroyQB();
  }
  console.log('make1kqb end');
}
// on document load
$(function(){
    $("#1kbtn").click(function(){
        make1kQB();
    }); 


});

</script>

How to check

chrome_2018-11-12_11-25-54

@mistic100
Copy link
Owner

mistic100 commented Nov 23, 2018

I made a rough test by overriding the method https://jsfiddle.net/zk9qt5fj/2/
It stills goes up, but more slowly (~5MB each run instead of ~10MB) there must some others references

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

Successfully merging this pull request may close these issues.

2 participants