tc39 / proposal-decorators

Decorators for ES6 classes
https://arai-a.github.io/ecma262-compare/?pr=2417
2.76k stars 105 forks source link

Are decorators strict mode code? #204

Open nicolo-ribaudo opened 5 years ago

nicolo-ribaudo commented 5 years ago

The spec defines strict mode for classes as follow:

https://tc39.github.io/ecma262/#sec-strict-mode-code [...] All parts of a ClassDeclaration or a ClassExpression are strict mode code.

This means that decorators should be parsed as strict. e.g. This should be an error;

with({}); // Ok

@(() => {
  with({}); // Error: with is not allowed in strict mode
})
class X {}

This behavior is fine if decorators are restricted to only classes: when an engine finds the @ token, it turns on strict mode. This is incompatible with possible future decorators for other productions:

// Suppose we have function decorators

@(() => { with({}); })
function X() {}

In this case, we don't know if the decorators is strict or not until we find the function keyword.


Possible solutions:

ljharb commented 5 years ago

Strict mode is determined lexically, not by the invocation. The code of a decorator defined outside the clsss isn’t inside the class just because @foo is inside it.

loganfsmyth commented 5 years ago

@ljharb The question here is specifically about decorators that are syntactically declared as part of a ClassDeclaration from the grammar standpoint, not related to invocation.

ljharb commented 5 years ago

Ah, thanks for clarifying. Certainly any decorator appearing inside a class body is in strict mode - but the question is about when the decorator is attached to something that may or may not be strict.

The options seem to be:

  1. inline decorator functions are in whatever mode they’re defined in, unrelated to the mode of the thing they’re decorating
  2. inline decorator functions are always strict, unrelated to the mode of the thing they’re decorating
  3. similar to non-simple parameter lists, inline decorator functions would be strict, but would throw if the thing they’re decorating turns out to be sloppy

The first is more intuitive to me, but the second aligns more with the “new constructs should be strict” mentality.

loganfsmyth commented 5 years ago

Totally agree, the first and second both seem totally acceptable to me.

erights commented 5 years ago

Third seems better to me, but second is acceptable.

I dislike the first because it would cause us to invest more effort into engineering new features for sloppy mode. The best perspective on sloppy mode is that exists only to be an ES3 compatibility mode. No code written since ES5 should have been written in sloppy mode. To the degree that we continue to backport new features into sloppy mode, we just mislead people about its purpose and the level of support they should expect.

littledan commented 5 years ago

The way I see it, class decorators are part of the class declaration/expression, even though they precede the class keyword, so i agree with the original post that they should have the same strictness as the extends clause. I was unaware that extends clauses were strict mode even in sloppy context--I plan to test this in engines before making a stronger judgement here.

littledan commented 5 years ago

OK, apparently I was just behind, and yes, four engines support the extends clause being in strict mode. I'd say decorators are in strict mode as well, then.

I don't think we need any spec text update here, as the text that @nicolo-ribaudo quoted says all we need. Maybe a note to clarify would be helpful.

nicolo-ribaudo commented 5 years ago

Do you think that possible future decorators on other constructs should always be strict as well? Otherwise we will need infinite lookahead to determine whether a decorators is strict or not.

littledan commented 5 years ago

This is a good point. I think it's likely that not all decorators constructs will be strict in the future. For example, we may have object literal decorators which will be usable entirely in sloppy mode. It would be extremely odd if decorators were in strict mode in that case. This seems to point to decorators taking the surrounding mode for their evaluation.

littledan commented 5 years ago

@erights expressed concern about decorator expressions being evaluated in sloppy mode because it would mean we're continuing to invest effort in sloppy mode features. But, my understanding of our 1JS policy was specifically that we are trying to keep things parallel between strict and sloppy mode. So disallowing object decorators in sloppy mode would be a surprising break from that.

erights commented 5 years ago

Or to take option 3 of @ljharb 's list at https://github.com/tc39/proposal-decorators/issues/204#issuecomment-453763265 :

This avoids lookahead, in addition to its other advantages https://github.com/tc39/proposal-decorators/issues/204#issuecomment-453789731

erights commented 5 years ago

I do not subscribe to the so-called 1js policy and I do not agree with it. There is no committee-wide consensus on this policy. I have consistently resisted adapting new features so that they also meant something reasonably parallel in sloppy mode. Although, in each case, I reluctantly agreed not to block, I never agreed with the 1js principle.

littledan commented 5 years ago

Oh, I see, I wasn't there for that part of the history.

I'm not a huge fan of that third option, but given the uptake of modules, it seems like not the end of the world. Presumably we're talking about an early error.

erights commented 5 years ago

Yes, early error. Exactly as for non-simple parameter lists.

littledan commented 5 years ago

OK, that's specified in #207. Any reviews would be welcome.

loganfsmyth commented 5 years ago

Is this really not considered excessively aggressive? Decorators will probably be pretty common in a lot of contexts, even with CommonJS modules. Requiring all users to add use strict seems pretty frustrating for users, especially since 99% of the time strictness won't matter for their usecase.

ljharb commented 5 years ago

doesn't this only apply when a decorator function is declared inline?

nicolo-ribaudo commented 5 years ago

No, the current patch will also throw for @foo

ljharb commented 5 years ago

Ah, then I think we should re-evaluate that. It certainly should be possible to decorate a sloppy mode function.

littledan commented 5 years ago

How would you resolve the lookahead concern without making the evaluation of the decorator itself sloppy mode? Not all future decorator constructs will be on strict things.

nicolo-ribaudo commented 5 years ago

My preferred option from https://github.com/tc39/proposal-decorators/issues/204#issuecomment-453763265 is 1, followed by 2

littledan commented 5 years ago

@erights expressed a concern about the first option in https://github.com/tc39/proposal-decorators/issues/204#issuecomment-453789731 , but I couldn't tell if it was a fatal concern.

Does anyone have strong concerns about making the evaluation of the decorator match the context, rather than the class?

littledan commented 5 years ago

Option 1 is drafted in #209.

ljharb commented 5 years ago

It was weird to have a parameter list be in a different mode than the function itself, but would it be weird to have a decorator be auto-strict, regardless of the mode of the context or the function being decorated? That would allow its use in sloppy mode and in sloppy functions, while creating more places that code was auto-strict.

littledan commented 5 years ago

The reason I am hesitant about that option is because some future decorated constructs (e.g., function decorators or object decorators) might not make sense to be in strict mode at all (unless they are in a strict context). It would be expensive to look ahead and see what's being decorated. Basically, this is the issue that @nicolo-ribaudo opened with.

ljharb commented 5 years ago

An object decorator indeed wouldn’t have a mode - although the context would - but a function defined inside a decorator expression would still need one (either from context, its own pragma, or auto-strict). It makes sense to me that new “bodies” are auto-strict, like Modules and classes, and that decorator “bodies” would be auto-strict too. (meaning that you don’t have to care what’s being decorated as long as you know you’re “inside” an @)

littledan commented 5 years ago

@ljharb I'm not sure what you're suggesting. In a sloppy context, do you think object decorators should have their expression evaluated in strict mode or sloppy mode?

ljharb commented 5 years ago

Option 1 above would say sloppy, option 2 strict. Either makes sense to me.

littledan commented 5 years ago

OK, it sounds like this thread is leaning towards Option 1. I'll merge that (since it at least reflects provisional consensus better than the current state), and let's keep discussing the issue here.

pabloalmunia commented 5 years ago

Other point of view: now into class body we cannot include a 'sloppy' function, but we can include a non-strict function with a simple Object.defineProperty():

`js function nonStrict () { with({}); }

class K { stricted () { // with({}); // Throw an error } }

Object.defineProperty( K.prototype, 'nonStrict', { value : nonStrict, enumerable : false, writable : true, configurable : true } );

const k = new K(); console.log( k.nonStrict() ); console.log( k.stricted() ); `

If we can include a sloppy function into a class with defineProperty, a decorator can be a sloppy function and replace or add method with sloppy functions.

erights commented 5 years ago

@erights expressed a concern about the first option in #204 (comment) , but I couldn't tell if it was a fatal concern.

It is not a fatal concern.

erights commented 5 years ago

Is this really not considered excessively aggressive? Decorators will probably be pretty common in a lot of contexts, even with CommonJS modules. Requiring all users to add use strict seems pretty frustrating for users, especially since 99% of the time strictness won't matter for their usecase.

@loganfsmyth No more than a handful of people understand the semantics of sloppy mode. No one understands these semantics when they are tired. No one should assume readers of their code understand sloppy semantics. No new code should be sloppy. When this requires saying 'use strict'; people should say 'use strict';

bakkot commented 5 years ago

@ljharb etc:

  1. similar to non-simple parameter lists, inline decorator functions would be strict, but would throw if the thing they’re decorating turns out to be sloppy

Unless I'm misunderstanding something, this does not accurately describe how non-simple parameter lists behave. The error is not for non-simple parameter lists on sloppy functions: (function(a = () => { with({}); }) {}) is legal. (Otherwise the "extra scope for parameter expressions" thing which has caused such headaches (e.g. https://github.com/tc39/ecma262/pull/1046) would not have mattered.) Rather, the error is for non-simple parameter lists on functions with a 'use strict' directive: (function(a = 0){'use strict';}) is an Early Error even when contained in strict mode code.

I'm happy with the approach taken in #209, just wanted to note that the option described above and implemented in #207 would not have precedent in how this issue is handled for non-simple parameter lists.

littledan commented 4 years ago

It looks like we came to a solid conclusion with https://github.com/tc39/proposal-decorators/pull/209 . We should pull this logic forward as the spec text for the new decorators proposal is drafted.

pzuraq commented 9 months ago

Reviewing issues now, just realized that this was not pulled forward into the revised stage 3 spec. Do we still feel like decorators should take on the strictness of the lexical environment that they are defined in?

erights commented 9 months ago

I see no reason for decorators to be legal in sloppy code. They should be a strict-only feature.

ljharb commented 9 months ago

That would mean only that you can’t decorate a class that’s defined in a sloppy context, since class bodies are always strict - what’s the benefit of that restriction?

erights commented 9 months ago

Specifically on decorating a class, I agree that decorations on the class should be considered for the purposes to be syntactically part of the class, therefore strict, and therefore consistent with "Decorators should be a strict-only feature".

But there are also decorator definitions, and coming up decorators for non-classes, like functions. For all of these...

As Brendan used to say "let's stop polishing a turd". Sloppy mode really still exists only to be an EcmaScript-3 compatibility mode. We have wasted too much time retrofitting new language features into it. New features are only used by new code. At this point, all new code should be strict.