Snugug / eq.js

Lightweight JavaScript powered element queries
http://eqjs.io/
MIT License
527 stars 34 forks source link

Uncaught TypeError: Cannot read property 'value' of undefined #66

Closed FezVrasta closed 8 years ago

FezVrasta commented 8 years ago

If I use eqjs.query(nodes, true);, where nodes is an array with a single node inside, I get:

Uncaught TypeError: Cannot read property 'value' of undefined

This happens because of this part of code:

        for (j = 0; j < eqPtsLength; j++) {
          var current = eqPts[j];
          var next = eqPts[j + 1];
          eqStates.push(current.key);

          if (j === 0 && objWidth < current.value) {
            eqState = null;
            break;
          }
          else if (next.value === undefined) {

It does: eqPts[j + 1]

But my element has a single eq-pts (data-eq-pts="a:0"), this makes next undefined, and throws an error when the script tries to read next.value

I'm not sure if I am doing something wrong or what... On top of my mind I would just make the condition like this:

else if (next  === undefined || next.value === undefined) {

Any idea?

FezVrasta commented 8 years ago

ping @Snugug

is this project still maintained? If not, may you please state it in the README so someone can adopt it?

Thanks.

jonscottclark commented 8 years ago

@FezVrasta, @Snugug has been responsive in the past, and I'm sure he will get around to checking out this bug eventually.

As far as this issue is concerned, I think the fix, as it relates to the error you receive, may not be that simple...

I encountered this error myself and applied your fix, but then it errors here: https://github.com/FezVrasta/eq.js/commit/3378b4421f7d43cd075d2305d348c8a9dddec78a#diff-f7356f20b0494347f9d4d2f882ff8225R174

Because next is undefined, so next.key will be undefined also.

Anyways.. I digress. In my case, I got this error when I had a div with eq.js attached to it, that when matched, was given the Bootstrap mixin @make-row, which essentially gave it clearfix and some negative left & right margins. Its children divs would then know they have enough room to become columns. It worked on initial page load, but then anytime I crossed the threshold, either up or down, I got this error on every resize event.

With some trial and error, I found that just changing the clearfix mixin made it work. The Bootstrap mixin is like so:

  &:before,
  &:after {
    content: " "; // 1
    display: table; // 2
  }
  &:after {
    clear: both;
  }

And my other clearfix mixin from Scut was like so:

  &:after {
    content: "";
    display: table;
    clear: both;
  }

Simply having a defined :before pseudoclass broke it.

After getting my implementation working with the 2nd mixin, I tried adding &:before { content: ""; } to the 2nd mixin and it broke again. So I really have no idea what's going on, hopefully from this explanation you might know why it's tripping up here, @Snugug ?

Snugug commented 8 years ago

I'll need to dig in to this as to why that specific implementation isn't working. The Bootstrap clearfix mixin seems pretty outdated. The obvious fix is to check to ensure that any time I'm checking for a property that that and the object I'm checking are defined.

Sorry for the delays, crazy at work. Planning some time to work through all of my FOSS stuff over the next few weekends.

jonscottclark commented 8 years ago

At first I thought maybe the negative margins, or not being cleared and having 0 height, or some race condition because of a change in dimensions at the eq point might have caused it.. so weird that the presence of :before would do it.. I haven't tried it in complete isolation though. Maybe having a pseudoelement at page load would make it work, and maybe the addition/removal of it makes it break? I really don't know yet. Hopefully whatever fix you come up with covers any other weird situations. Thanks again @Snugug :)