Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decorator standard library #54

Open
littledan opened this issue Feb 11, 2018 · 13 comments
Open

Decorator standard library #54

littledan opened this issue Feb 11, 2018 · 13 comments

Comments

@littledan
Copy link
Member

At the January 2018 TC39 meeting, during the decorators presentation, the committee discussed a possible decorators standard library.

Would it make sense to sketch out, at an explainer level, what such a library might contain? Or, are we entirely content with leaving this to ecosystem-level solutions (e.g., core-decorators)?

At the time, some people in committee seemed to be advocating for a decorators standard library to be part of the this proposal, while others were suggesting that it would make more sense to wait until decorators are shipped for a while and see what JavaScript programmers find useful for themselves.

A compromise might be to aim to develop a Stage 1 proposal for a decorator standard library while this decorators proposal is at Stage 2 or 3. Even if it stays at Stage 1 for a while, such a standard library might be a useful design for polyfills and gain usage organically that way.

@mbrowne
Copy link
Contributor

mbrowne commented Feb 24, 2018

Just putting this here for the future...one thing I'd like to see in a standard library is a decorator to create a soft-private field or method. It would be simple for anyone to implement on their own, but the naming could get confusing if different people call it different things in different codebases, e.g. @inspectable, @reflectable, @softPrivate...It would be good for the community to pick one and make it standard. The same probably goes for some other very common decorators I'm guessing.

@bmeck
Copy link
Member

bmeck commented Feb 28, 2018

I would prefer this be left to the ecosystem for everything that doesn't map to property descriptor fields for now (configurable/writable/enumerable in particular).

@ljharb
Copy link
Member

ljharb commented Feb 28, 2018

What about seal/extensible/frozen, and decorator placement, let alone autobinding? I think it would be a big mistake to leave a gap like this in the feature and cause ecosystem fragmentation for known-common use cases.

@bmeck
Copy link
Member

bmeck commented Feb 28, 2018

@ljharb I have concerns about those and see them as non-trivial.

@ljharb
Copy link
Member

ljharb commented Mar 1, 2018

What concerns?

@bmeck
Copy link
Member

bmeck commented Mar 2, 2018

For:

@freeze
class A {}
@seal
class A {}

Extending those classes would not allow adding properties, but does allow adding private fields. This seems something to at least discuss. WeakMaps get around frozen stuff as well so it might not be so controversial. Also timing is something I'd like to discuss here, if it should freeze during the constructor or when new.target's constructor finishes?

Also, those feel kind of odd without Object literal decorators to me.

For autobinding, I think public fields kind of remove most of the usefulness of that decorator. I could be wrong, but would want to discuss if it is giving reasonable gains to put into the standard library.

@mbrowne
Copy link
Contributor

mbrowne commented Mar 2, 2018

Specifically regarding autobinding, see my PR where I explained its advantages over arrow functions in class fields...specifically the "Notes on the @bound decorator" section: https://github.com/tc39/proposal-decorators/pull/59/files

@bmeck
Copy link
Member

bmeck commented Mar 2, 2018

@mbrowne while I agree that there are cases where arrow functions are not ideal, I'm not convinced that @bound is always preferable to arrow functions either.

@ljharb
Copy link
Member

ljharb commented Mar 2, 2018

This is true of builtins already; all the collections, as well as Promise, and RegExp’s internal state via .compile - it’s been discussed previously and i don’t think there are any problems there.

Class decorator timing is already determined iirc, altho i don’t recall it off the top of my head.

Object literal decorators seem super helpful and an important followon, but that’s imo even more of a reason to ship standard decorators now - then, if any changes are needed for object literal decorators, we can ship those instead of having not-fully-compatible implementations in the wild.

As for bound being preferable to arrow functions, i can dredge up dozens of issues with enzyme (the canonical react testing lib) that demonstrate that arrow functions in class properties are a general antipattern (one the airbnb guide will be banning once an eslint rule is created, fwiw)

@bmeck
Copy link
Member

bmeck commented Mar 2, 2018

Since there is disagreement on these, I want to discuss it. I'm open to adding things as standard library features, but don't think doing so on first iteration is necessary. We also haven't even talked about where the references to these decorators would live.

@rbuckton
Copy link
Collaborator

rbuckton commented Mar 2, 2018

@freeze
class A {}

@bmeck, at the last meeting I discussed this with @littledan about a solution that would work. I put together a rough outline of the approach in a gist.

@rbuckton
Copy link
Collaborator

rbuckton commented Mar 2, 2018

I think there might be value in something like an @conditional decorator that the host can exercise some control over, similar to how the C# ConditionalAttribute behaves at compile time. We've considered something like that for TypeScript in the past as well (as some kind of design-time-only decorator).

@littledan
Copy link
Member Author

Extending those classes would not allow adding properties, but does allow adding private fields. This seems something to at least discuss.

I'm not sure how this discussion belongs in the decorators proposal exactly. This is more a property of what Object.freeze does on objects with private fields. We've discussed this in the private fields proposal; the behavior here is analogous to internal slots, and I don't see an alternative that preserves the properties of Proxies that the committee considers important. (@gsathya has also raised the question recently on Twitter.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants