Add a placement: 'none' for conditional properties

[Jamesernator]

Currently if placement is not one of "static", "prototype" or "own" then decoration throws an error. It would be nice if we could have conditional decorators that can avoid creating the property at all given some condition.

Example: expose hidden values when doing tests.

// if-test.js during test
export default descriptor => descriptor

// if-test.js normally
export default descriptor => descriptor.placement = 'none'

// component.js
import ifTest from "if-test"

class MyWebComponent extends HTMLElement {
    @ifTest
    get internals() {
        return {
             shadowRoot: this.#shadowRoot,
             pauseTime: this.#pauseTime,
        }
    }
}

Example 2. progressive feature detection without Object.defineProperty

const when = condition => descriptor => {
    if (!condition) {
        descriptor.placement = 'none'
    }
}

class Point {
    @when(Symbol.futureFeature)
    [Symbol.futureFeature]() {
        ...
    }
}

[littledan]

We explored another API for this sort of thing in #44 . That issue uses key: undefined instead, which also lets you use a field with an initializer to execute code at a related time. So far, we've decided to leave this for "v2" and out of the current decorators proposal; should we reconsider?

[Jamesernator]

As in the when from above could be defined like?:

const when = condition => descriptor => {
    if (!condition) {
        descriptor.key = undefined;
    }
}

I don't think it would make a huge difference which was included as the use cases I'd imagine for them would be covered by either proposal. Although I do think including some mechanism would be preferable as the first example would actually be impossible as outside of the class this would just fail:

class MyComponent extends HTMLElement {
    ...
}

if (isTest) {
    Object.defineProperty(MyComponent.prototype, 'internals', {
        get() {
             return {
                 // SyntaxError: !
                 shadowRoot: this.#shadowRoot,
             }
        },
    })
}

Certainly the situation isn't dire, one could easily just have if (!isTest) { throw new Error(Can't access internals outside test) }, although perhaps decorators have better static analysis properties that minifiers and such can use to strip out things like @isTest in production.

[littledan]

One use case that the proposal in this issue wouldn't cover, but the one in #44 would cover, is executing some code per instance. That doesn't come up in your example, though.

[littledan]

We discussed this possible feature in a call among decorators champions. @wycats argued that it should not be in the initial decorators proposal, as we should be generally encouraging decorators authors to use decorators for things which will end up adding something related to what the decorated field or method is doing. For this reason, the feature is likely to wait until a follow-on proposal.

[littledan]

Another mitigating factor is that class decorators can remove class elements. So, for example, the @call decorator could be a class decorator factory which takes a static method property key as an argument, and then go and delete it then in a way which doesn't have runtime overhead. Due to these things together, it seems like we can leave this feature for a future proposal.

[mbrowne]

I just came across a use case for this...I'm playing around with lit-element and trying to get their @property decorator working with the latest decorators proposal and Babel 7 (currently it's not working in Babel 7 even with the legacy decorators option; I think that's a Babel issue). Their current implementation of the decorator is here: https://github.com/Polymer/lit-element/blob/master/src/lib/decorators.ts#L53-L61.

The most straightforward way to update this decorator to work with the current proposal is to use a class finisher:

export function property(options) {
    return elementDescriptor => ({
        ...elementDescriptor,
        finisher(klass) {
            klass.createProperty(elementDescriptor.key, options)
        }
    })
}

...but that isn't sufficient because the instance property conflicts with what createProperty() is doing. So it would be nice to just be able to say "don't create a field at all, just run the finisher".

I'm guessing that this use case isn't sufficient to motivate changing the plan of addressing this in a follow-on proposal, but I thought I'd share it anyway since it's a real-world example.

Arguably a more proper solution here is to not call createProperty() from the decorator at all. createProperty() is creating a prototype property which could instead be created in the decorator function by changing placement to prototype and creating the necessary getter and setter. There would be one downside of that though: the non-decorator and decorator APIs could no longer share the same code. Many library authors like to provide an equivalent API without decorators for those who prefer to be more explicit.

OTOH, I think this is a resaonble argument for waiting on this feature:

@wycats argued that it should not be in the initial decorators proposal, as we should be generally encouraging decorators authors to use decorators for things which will end up adding something related to what the decorated field or method is doing. For this reason, the feature is likely to wait until a follow-on proposal.