gss / engine

GSS engine
http://gss.github.io
MIT License
2.87k stars 103 forks source link

Exception using GSS with Polymer custom elements #194

Open sparkofreason opened 9 years ago

sparkofreason commented 9 years ago

See https://gist.github.com/sparkofreason/164d7e9a7c4cad833662. Loading this page gives this exception:

Uncaught TypeError: value.slice is not a function
Query.pad @ gss.js:22183
Query.pair @ gss.js:22238
Query.repair @ gss.js:22157
Engine.$events.commit @ gss.js:15800
Engine.triggerEvent @ gss.js:16056
Engine.commit @ gss.js:15624
Engine.solve @ gss.js:15572
Document.$$events.compile @ gss.js:15056
Engine.triggerEvent @ gss.js:16056
Engine.compile @ gss.js:15786
Document.$$events.DOMContentLoaded @ gss.js:15144
Engine.triggerEvent @ gss.js:16056
Engine.handleEvent @ gss.js:16107
paulyoung commented 9 years ago

Hey @sparkofreason!

I see that you're using HTML imports. Are you using Polymer or something similar?

sparkofreason commented 9 years ago

I used Chrome for the example I posted, though I could put together one that will work in Firefox if that helps. <paper-slider> is one of the Polymer elements, and loads Polymer under the hood. Chrome implements HTML imports natively, Polymer shims them for FF.

sparkofreason commented 9 years ago

FWIW - I seem to be able to avoid the exception by change line 22182 in gss.js to read as:

} else if (!(value instanceof HTMLElement ) && (value != null ? value.splice : void 0)) {

Not clear whether that's the proper fix.

Inviz commented 9 years ago

Hey. It's strange. Polymer adds splice method to elements or something? Your fix kinda does the job. I'll make a cleaner one later today.

mtaufen commented 9 years ago

The corresponding Coffescript is here: https://github.com/gss/engine/blob/3f6a5922420ce94fcc2db60a9fd1eba5b443f6a5/src/engine/Query.coffee#L638

It might be worth looking into why GSS checks for the splice method on value, then calls the slice method instead. A type check might be more appropriate than a method existence check. My best guess right now is that the authors did not anticipate both push and splice being exposed on anything but Array. It looks like certain Polymer elements (<template>, for example), however, expose several methods conventionally on Array to facilitate data binding: https://www.polymer-project.org/1.0/docs/devguide/templates.html

It's worth noting that } else if (value != null ? Array.isArray(value) : void 0) { avoids the exception. Not that simply avoiding the exception really means much.

I modified gss.js to get some output from the Query.prototype.pad function. I'm not sure what the purpose of the Query.prototype.pad function is yet, so I can't confidently suggest a fix... but maybe someone will find this helpful.

Modifications:

Query.prototype.pad = function(value, length) {
    console.log("Query.prototype.pad: Value: " + value + " Length: " + length);
    console.log(value);
    var i, result, _i;
    if (value && !value.push) {
      result = [];
      for (i = _i = 0; 0 <= length ? _i < length : _i > length; i = 0 <= length ? ++_i : --_i) {
        result.push(value);
      }
      result.single = true;
      return result;
    } else if (value != null ? value.splice : void 0) {
      console.log("Value had .push, and also has .splice.");
      console.log("Sliced: " + value.slice());
      console.log(value.slice());
      return value.slice();
    } else {
      return value || [];
    }
  };

Output:

gss.js:22174 Query.prototype.pad: Value: [object HTMLDivElement] Length: 1
gss.js:22175 
div#d1
gss.js:22174 Query.prototype.pad: Value: undefined Length: 0
gss.js:22175 undefined
gss.js:22174 Query.prototype.pad: Value: [object HTMLDivElement] Length: 1
gss.js:22175 
div#d2
gss.js:22174 Query.prototype.pad: Value: undefined Length: 0
gss.js:22175 undefined
gss.js:22174 Query.prototype.pad: Value: [object HTMLDivElement] Length: 1
gss.js:22175 
div#d1
gss.js:22174 Query.prototype.pad: Value: undefined Length: 0
gss.js:22175 undefined
gss.js:22174 Query.prototype.pad: Value: [object HTMLDivElement] Length: 1
gss.js:22175 
div#d2
gss.js:22174 Query.prototype.pad: Value: undefined Length: 0
gss.js:22175 undefined
gss.js:22174 Query.prototype.pad: Value: [object HTMLDivElement] Length: NaN
gss.js:22175 
div#d1
gss.js:22174 Query.prototype.pad: Value: undefined Length: 0
gss.js:22175 undefined
gss.js:22174 Query.prototype.pad: Value: [object HTMLElement] Length: NaN
gss.js:22175 
paper-slider#slider.x-scope.paper-slider-0
gss.js:22185 Value had .push, and also has .splice.
gss.js:22186 Uncaught TypeError: value.slice is not a function
Inviz commented 9 years ago

I think I fixed this on ranges2 branch already

https://github.com/gss/document/commit/3effd663466771a7c61d310fc54e0a32ac60328f#diff-a30c2d83dd31eedc8e0aaeb7a92a62bbR3484

mtaufen commented 9 years ago

Nice. If the guard is in fact a duck-typing check for an Array, it might be worth switching to Array.isArray(value) to improve forward compatibility with custom DOM elements. Otherwise, looks good :).