isStage1Descriptor: false positive, if passed a function

[buschtoens]

If you pass a function() {} as the only argument to a decorator, isStage1Descriptor returns a false positive.

// handler is a function
const myDecorator = decoratorWithRequiredParams((desc, [handler]) => desc);

// breaks
@myDecorator(function() {})
class Foo {}

// works
@myDecorator(() => {})
class Bar {}

ember-decorators/packages/utils/addon/decorator.js

Lines 31 to 35 in f6259d7

	} else if (possibleDesc.length === 1) {
	let [target] = possibleDesc;
	return typeof target === 'function' && 'prototype' in target;
	}

Do you think that there is any way to reliably tell a function() {} apart from a class {}?

Funny coincidence, that "How Does React Tell a Class from a Function?" by Dan Abramov is trending on HN now. 😄

[buschtoens]

Taking the babel class transforms into account, I don't think there is any way to tell the difference between a function() {} and a class {}.

We could either do it on a best effort basis, which will still fail in certain cases, or just outright "ban" users from passing a single function as a parameter. Arrow functions or function() {} stripped of their prototype would be fine.

Some ideas for heuristics:

• We can check the length of the prototype chain. function() {} and class {} have the same length, but derived classes have longer chains: One extra link per extends.
• We can check whether target.prototype instanceof Ember.Object. This would reliably detect all classes that extend Ember framework classes.

We could also ship an extra babel transform, that transforms all decorator invocations with one singular parameter to be "tagged".

@myDecorator(singleParam)
class Foo {}

import { tag } from '@ember-decorators/utils/tag';

@myDecorator(tag(singleParam))
class Foo {}

const tagged = new WeakSet();

export function tag(param) {
  if (typeof param === 'function') {
    tagged.add(param);
  }
  return param;
}

export function isTagged(param) {
  return typeof param === 'function' || tagged.has(param);
}

This would be 100 % safe. But is it overkill?

Do we only need this transform for Babel 6 or also for Babel 7?

Seeing that this check was added with v3.1.0 and that version also requires Babel 7, that question is answered.

[buschtoens]

I would be up for writing such a transform, btw.

[buschtoens]

If anyone else also runs into this, these are the ways out:

• Use an arrow function instead.
• Pass a second, to be ignored argument, like undefined, to defuse the arguments.length === 1 check.
• Strip off the prototype from the function before you pass it.

[pzuraq]

Yeah, not sure if it's really worth the effort given stage 1 decorators are on the way out. I think passing an arrow function is the best idea, though that could be tricky if the function needs to have this rebound.

[buschtoens]

Passing an arrow function also has another gotcha: Depending on your targets.js, it might get transpiled to a regular function() {}. I was very confused for a moment, why my app was failing on staging, but not in dev mode. 😄