makeArray( null ) about fizzy-ui-utils HOT 8 CLOSED

@shogunsea Thanks for reporting this issue. I'm having trouble understanding the bug. Both these arguments return same values in v2.0.3 and v2.0.4.

// v2.0.3
fizzyUIUtils.makeArray( null )
// => [ null ]
fizzyUIUtils.makeArray( document.querySelectorAll('foo') )
// => []

// v2.0.4
fizzyUIUtils.makeArray( null )
// => [ null ]
fizzyUIUtils.makeArray( document.querySelectorAll('foo') )
// => []

from fizzy-ui-utils.

This may be an older Safari bug. Please chime in if you know which browsers this occurs in.

from fizzy-ui-utils.

They occurs in PhantomJS environment both windows and osx

from fizzy-ui-utils.

Is it actually intended behavior - to make arrays of nulls, if the passed to .makeArray? Or, then in line 201

    var dataAttrElems = document.querySelectorAll( '[' + dataAttr + ']' );
    var jsDashElems = document.querySelectorAll( '.js-' + dashedNamespace );
    var elems = utils.makeArray( dataAttrElems )
      .concat( utils.makeArray( jsDashElems ) );

If no coressponding elements found - you actually create array of two nulls - which is causing errors of elem.getAttribute later in line 209 - null does not have method getAttribute - so it causes errors in my case.

Maybe just add another check if (obj) in makeArray before pushing it? to avoid creating array of nulls or undefined or else:

// line 68
  } else if (obj) {
    // array of single index
    ary.push( obj );
  }
  return ary;
};

I think that the problem is that some browsers or phantomjs builds (linux and osx) return null rather that empty array, if document.querySelectorAll did not find anything. So you always expect at least empty array. Other way is to check elements are really found before making array, for example

var dataAttrElems = document.querySelectorAll( '[' + dataAttr + ']' );
var jsDashElems = document.querySelectorAll( '.js-' + dashedNamespace );
dataAttrElems = dataAttrElems || []; // fixes strange phantomjs results
jsDashElems  = jsDashElems  || []; // same

// and only then call .makeArray
var elems = utils.makeArray( dataAttrElems )
      .concat( utils.makeArray( jsDashElems ) );

from fizzy-ui-utils.

I think that the problem is that some browsers or phantomjs builds (linux and osx) return null rather that empty array

Aaah, I was not aware of this behavior. I'm having trouble pinning down where & when querySelectorAll('foo') will return null.

But, to your question, makeArray( null ) should return an empty array [], as should makeArray( undefined ) or just makeArray(). This is not the current behavior with v2.0.4. I'll follow up with a new release that changes this behavior

from fizzy-ui-utils.

Hi @desandro, could you say if your last commit should fix this issue? Because the last commit is not included in 2.0.5, could you just in case publish 2.0.6 with the fix?

I found this github issue becuase I spend a few long hours on checking sentry in one of our products and found that one error is still reported (reported many times in the last few months). Error is reported from the "minified code" so it took me a bit more time to find it in the source code, finally I got it, it's fizzy-ui-utils code:

utils.htmlInit = function( WidgetClass, namespace ) {
  utils.docReady( function() {
    var dashedNamespace = utils.toDashed( namespace );
    var dataAttr = 'data-' + dashedNamespace;
    var dataAttrElems = document.querySelectorAll( '[' + dataAttr + ']' );
    var jsDashElems = document.querySelectorAll( '.js-' + dashedNamespace );
    var elems = utils.makeArray( dataAttrElems )
      .concat( utils.makeArray( jsDashElems ) );
    var dataOptionsAttr = dataAttr + '-options';
    var jQuery = window.jQuery;

    elems.forEach( function( elem ) {
      var attr = elem.getAttribute( dataAttr ) ||
        elem.getAttribute( dataOptionsAttr );
      var options;
      try {
        options = attr && JSON.parse( attr );
      } catch ( error ) {
        // log error, do not initialize
        if ( console ) {
          console.error( 'Error parsing ' + dataAttr + ' on ' + elem.className +
          ': ' + error );
        }
        return;
      }
      // initialize
      var instance = new WidgetClass( elem, options );
      // make available via $().data('namespace')
      if ( jQuery ) {
        jQuery.data( elem, namespace, instance );
      }
    });

  });
};

These lines

      var attr = elem.getAttribute( dataAttr ) ||
        elem.getAttribute( dataOptionsAttr );

Reported error (after minification)

'undefined' is not a function (evaluating 't.getAttribute(a)')

which means this one line

elem.getAttribute( dataAttr )

In Sentry we see that this is reported for:

• Chrome 54.0.2840
• Firefox 50
• Chromium 53.0.2785
• PhantomJS 1.9.7 💃
• Safari 5.1.9
• IE 11 🗡

Many thanks!

from fizzy-ui-utils.

@hinok Thanks for the ping! I've released v2.0.6 with these changes. Please update & let me know if you continue to run into issues.

from fizzy-ui-utils.

I'm closing this issue as fixed. Please chime in if you have run into this issue after v2.0.6

from fizzy-ui-utils.