4.1 doesn't work with Typescript

[Hotell]

The new implementation of API on prototype doesn't work with Typescript which is very unfortunate.

Problem is probably in Typescript implementation of extends.

It works only with old skate js API's ( define and definition via object )

Short description of possible issue:

• problem starts here ( within connectedCallback ): https://github.com/skatejs/skatejs/blob/master/src/api/component.js#L186

• which calls syncPropsToAttrs(this), where this is the custom element constructor. Problem is that there is no props static property on it so this call (https://github.com/skatejs/skatejs/blob/master/src/api/component.js#L34) will return undefined and when you call undefined.forEach well it throws...

Here is the repo which reproduces the error:
https://github.com/Hotell/skate-and-raw-WC

and here is skate component https://github.com/Hotell/skate-and-raw-WC/blob/master/src/app/skate-wc/yo-smile/YoSmile.tsx

just hit yarn || npm i to install dependencies, then yarn start || npm start and open devTools, to debug it.

[vvakame]

related issue microsoft/TypeScript#3610

[Hotell]

also just one more note, older versions of skate did work with Typescript

[vascofernandes]

Would be nice to have this fixed.
I always go TS first these days.

[andreawyss]

@Hotell please check it PR #932 (Cache propConfigs returned from static get props)
fixes this issue that you are having with SkateJs 4.1 and TypeScript
If it does than have @treshugart review my changes.
Thank You

[vvakame]

I made work around. check this change.

[Hotell]

nice @vvakame! ,

you don't need to import tslib manually, just hit "importHelpers": true, in tsconfig.json and just override the global._extends as you do already

[vvakame]

@Hotell I tried it now. but it doesn't work as expected.
following code generated.

	"use strict";
	var tslib_1 = __webpack_require__(1);
	__webpack_require__(2);
	var skate = __webpack_require__(3);
	window.__extends = function (d, b) {
	    Object.setPrototypeOf(d, b);
	    var __ = function () { this.constructor = d; };
	    d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
	};
	var CountUpComponent = (function (_super) {
	    tslib_1.__extends(CountUpComponent, _super);
	    function CountUpComponent() {
	        return _super.apply(this, arguments) || this;
	    }

I feel that the generated code changes when I tried importHelpers: true before...

[Hotell]

yeah doesn't work for me as well :( so we have to manually import tslib and then override __extends

[andreawyss]

@Hotell I would not change the prototype of your derived class to be the prototype of your base class. You need to maintain the correct prototype chain in place.
Try to use this code that uses getOwnPropertyNames:

`
var __extends = (this && this.__extends) || function (d, b) {

Object.getOwnPropertyNames(b).forEach(function(p) {
  if (-1 === ['length', 'name', 'prototype'].indexOf(p)) {
    d[p] = b[p];
  }
});

function __() {
  this.constructor = d;
}

d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());

};
`

[Hotell]

that doesn't work @andreawyss

for now I'm rather overriding typescript __extends with babel's _inherits.

Works like a charm, although yeah I'm not big fan of using setPrototypeOf and not just for perf reasons...

function _inherits(subClass, superClass) {
  if (typeof superClass !== "function" && superClass !== null) {
    throw new TypeError("Super expression must either be null or a function, not " + typeof superClass);
  }
  subClass.prototype = Object.create(superClass && superClass.prototype, {
    constructor: {
      value: subClass,
      enumerable: false,
      writable: true,
      configurable: true
    }
  });
  if (superClass) Object.setPrototypeOf ? Object.setPrototypeOf(subClass, superClass) : subClass.__proto__ = superClass;
}

(window as any).__extends = _inherits;

[andreawyss]

@Hotell I Yes, I like it.

When you tried my version did you check the one that skips over 'length', 'name', 'prototype' ?

Object.getOwnPropertyNames(b).forEach(function(p) { if (-1 === ['length', 'name', 'prototype'].indexOf(p)) { d[p] = b[p]; } });

[Hotell]

so I can add to docs how to skate 4.1 work with TS, WDYT @treshugart ?,

because I don't think that this will be fixed anytime soon within TS, because it would be a huge Breaking Change

[Hotell]

@andreawyss

When you tried my version did you check the one that skips over 'length', 'name', 'prototype' ?

Nope, but anyway, that's to much nitpicking magic for my taste, I think by using Babel's inherits method, the problem is solved at this moment. WDYT?

If @treshugart gives me a GO, I'll submit PR to docs

[andreawyss]

@Hotell I think you should give it a try.
It follows the original pattern of TypeScript __extends.

But yes, now I understand the reason why static class member should be enumerable=false and I agree that this is something that TypeScript will needs to address.
I like your solution better.

[treshugart]

@Hotell go for it. I think it's at least a good starting point for now, that matches the current state of affairs. Hopefully this will be addressed soon. This feels like a refactor to TS would solve this? cc @andreawyss

[andreawyss]

Yes, TS is still incorrectly generating static class member properties enumerable=true.
But at least then a class generated with TS can then be extended with ES6+Babel code.

[Hotell]

yup (breaking)change on TS side is needed

[DanielRosenwasser]

We'll be discussing this in the near future. This might be something to keep an eye out for in the 2.2 timeframe.

[treshugart]

Going to close this as it seems we've gotten as far as we can. As per #972 we're going to try Flow as well and see how that works out. We can always switch later after we've had experience trying both.

[vvakame]

FYI: microsoft/TypeScript#12488 this issue will be solved in TypeScript 2.2