loganfsmyth / babel-plugin-transform-decorators-legacy

A plugin for Babel 6 that (mostly) replicates the old decorator behavior from Babel 5
MIT License
817 stars 57 forks source link

clearing decorators array on methods even if no decorators are found #80

Closed sirrodgepodge closed 6 years ago

sirrodgepodge commented 6 years ago

Got bitten by this when developing babel-plugin-undecorate: https://www.npmjs.com/package/babel-plugin-undecorate

See corresponding comment here: https://github.com/sirrodgepodge/babel-plugin-undecorate/blob/master/index.js#L50

There's a devious (apparently quite edge-y) case that's wasn't covered by this lib where none of a class's methods have decorators but one or more does have an empty array in the "decorators" property.

If this case occurs, applyTargetDecoratorswon't be run on this method, which means that this decorators property will not get set to null (it will remain an empty array).

This means that if using Babel 6 (which many do since Babel 7 is still beta) "babel-plugin-transform-decorators" is then run subsequently (as it will be with the common babel-preset-stage-0) it will throw due to this check (arguably a bug there that there's no empty array check): https://github.com/babel/babel/blob/6.x/packages/babel-plugin-transform-decorators/src/index.js#L55

The code in this PR fixes this by clearing the decorators empty arrays on each method even if no actual decorators are found.

I'd add a UT but I'm afraid I would kinda need to disrupt the whole way you've been running tests as the only way that I know how to replicate is by modifying AST with another babel-plugin.

sirrodgepodge commented 6 years ago

yo suckas, merge plz :)

loganfsmyth commented 6 years ago

I think I'd rather fix this via https://github.com/babel/babel/pull/8314 since that's the real issue. null and [] should behave identically in this context and the fact that they don't is the real issue.

sirrodgepodge commented 6 years ago

thank you, learned something :)