New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hydrolysis errors if using ES6 classes #35

Closed
robdodson opened this Issue Jun 18, 2015 · 17 comments

Comments

Projects
None yet
@robdodson

robdodson commented Jun 18, 2015

I recently tried using an ES6 class for my element's definition and saw the following hydrolysis error when viewing the docs.

Failed to load source at: http://localhost:8080/components/quick-alert/quick-alert.html TypeError: Cannot read property 'forEach' of undefined
    at annotateElement (http://localhost:8080/components/hydrolysis/hydrolysis.js:1061:24)
    at Array.forEach (native)
    at Analyzer.annotate (http://localhost:8080/components/hydrolysis/hydrolysis.js:470:17)
    at http://localhost:8080/components/hydrolysis/hydrolysis.js:177:16

I'm wondering if this PR should fix the issue or if it still requires more work. cc @garlicnation

Element definition is here if you want to repro:

<link rel="import" href="../polymer/polymer.html">

<!--
An element providing a solution to no problem in particular.
Example:
    <quick-alert></quick-alert>
@group Seed Elements
@element quick-alert
@demo demo/index.html
@hero hero.svg
-->
<dom-module id="quick-alert">
  <style>
    :host {
      display: block;
      position: fixed;
      top: 0;
      left: 0;
      right: 0;
    }
    .alert {
      background-color: #4CAF50;
      color: white;
      padding: 16px;
      font-family: Roboto, Helvetica, sans-serif;
    }
    [hidden] {
      display: none;
    }
  </style>
  <template>
    <div class="alert" hidden="{{!shown}}">
      <content></content>
    </div>
  </template>
</dom-module>

<script>
  'use strict';

  class QuickAlert {
    get is() {
      return 'quick-alert';
    }

    get properties() {
      return {
        shown: {
          type: Boolean,
          value: false
        }
      }
    }
  }

  Polymer(QuickAlert.prototype);
</script>
@cadwmaster

This comment has been minimized.

cadwmaster commented Sep 8, 2015

Had the same error trying to fix the documentation and demo of the paper-dropdown-menu component

@bedeoverend

This comment has been minimized.

bedeoverend commented Oct 6, 2015

+1. Though I'm using a slightly different syntax, the one outlined in this article - I'm not sure if that makes a difference to the parser.

@ebidel

This comment has been minimized.

Member

ebidel commented Oct 6, 2015

Yea, I wouldn't use the syntax in #35 (comment). get properties() is returning a new properties object on every access! Instead, properties should be setup once at registration time. The syntax in the article is 💰

@cayblood

This comment has been minimized.

cayblood commented Nov 17, 2015

I'm declaring properties in beforeRegister() but still seeing this error. Anyone got this working?

@bedeoverend

This comment has been minimized.

bedeoverend commented Nov 18, 2015

Is there any update on this? Did the ES6 PR attempt to resolve?

I'm using ES6 in all my components and would love to be able to auto-generate docs, are there any examples / docs of ES6 syntax that is supported by hydrolysis?

@jgautheron

This comment has been minimized.

jgautheron commented Nov 24, 2015

The ES6 PR didn't solve this issue. I have the same error as @robdodson mentioned above: Cannot read property 'forEach' of undefined which is related to the element properties not being properly parsed.

This error is only a symptom, not the source of the issue, a quick fix silences the error but still the component page won't show.

Here's the sample code which fails:

class Foo {
  beforeRegister() {
    this.is = 'foo';
    this.properties = {
      cover: String,
      photo: String,
      username: String
    };
  }
}
Polymer(Foo);

Compiling the ES6 code through Babel doesn't help at all. Still fails, for the same reason.

@jgautheron

This comment has been minimized.

jgautheron commented Nov 27, 2015

I fixed it in my fork.
ES6 class parsing is just not implemented, so I implemented it quickly. I would have thrown a PR, but some tests are failing for no apparent reason and it could be cleaner (a separate elementFinder for ES6 classes). If someone is willing to help...

@efeminella

This comment has been minimized.

efeminella commented Dec 1, 2015

Any ETA or a roadmap for when ES6 class parsing will be natively supported by Hydrolysis, perhaps a PR from @jgautheron 's fork to get this going?

@jgautheron

This comment has been minimized.

jgautheron commented Dec 2, 2015

@RoXuS

This comment has been minimized.

RoXuS commented Dec 3, 2015

Hi jgautheron,

Thx for your PR.

It seems to be not good with behavior :

get behaviors() {
    return [
      ...,
    ];

The result :
capture d ecran 2015-12-03 a 09 29 49

Maybe my statement isn't good ?

@jgautheron

This comment has been minimized.

jgautheron commented Dec 3, 2015

You should declare them in beforeRegister with this.behaviors = [];.

@MeTaNoV

This comment has been minimized.

MeTaNoV commented Dec 3, 2015

and for listeners, this should work?

this.listeners = {
        "tap": "regularTap",
        "special.tap": "specialTap"
      };
@jgautheron

This comment has been minimized.

jgautheron commented Dec 3, 2015

Listeners are not shown in the iron-component-page, AFAIK for now only the following are displayed:

  • properties
  • methods
  • behaviors
  • events
@efeminella

This comment has been minimized.

efeminella commented Jan 11, 2016

Hey guys, any traction on this yet?

@justinfagnani

This comment has been minimized.

Contributor

justinfagnani commented Feb 11, 2016

I'm moving this bug to hydrolysis

@mbana

This comment has been minimized.

mbana commented May 1, 2016

Does this actually work now? If so, would someone please post an example. I've been trying to get it to work not realising it's a bug: the /demo/ and /test/ routes in polyserve work, the documentation page however doesn't.

@justinfagnani

This comment has been minimized.

Contributor

justinfagnani commented May 2, 2016

@mbana this bug is closed. You can continue the discussion at: Polymer/polymer-analyzer#221

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment