bublejs / buble

https://buble.surge.sh
MIT License
869 stars 67 forks source link

Properties for class methods are not set properly #197

Open ClassicOldSong opened 5 years ago

ClassicOldSong commented 5 years ago

ES6 classes methods should not be enumerable, but code transpiled with buble got these methods enumerable.

Things like

class aaa {
  bbb() {}
}

should be transpiled into:

var aaa = function aaa () {};

aaa.prototype.bbb = function bbb () {};
Object.defineProperty(aaa.prototype, 'bbb', {enumerable: false})

But the last line doesn't exist. So when trying to traversing the instance of class aaa will get bbb included, but it shouldn't appear. Hope this could be fixed soon.

ClassicOldSong commented 5 years ago

Workaround for this could be

class aaa {
  bbb() {}
}

Object.defineProperty(aaa.prototype, 'bbb', {enumerable: false})
mourner commented 5 years ago

Thanks for the report! Babel seems to respect enumerable: false here, but TypeScript curiously [transpiles in the same way](https://www.typescriptlang.org/play/#src=class%20A%20%7B%20b()%20%7B%7D%20%7D%0D%0A) as Buble currently. I wonder if this is an accepted tradeoff — e.g. could there be a performance implication to using defineProperty?

ClassicOldSong commented 5 years ago

There shouldn't be performance issues with defineProperty since setting the property is done while defining the class. Maybe we could file up the same bug report to TypeScript.

ClassicOldSong commented 5 years ago

Okay there's already an issue for TypeScript: https://github.com/Microsoft/TypeScript/issues/15038