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

Are decorators strict mode code? #204

Open
nicolo-ribaudo opened this issue Jan 12, 2019 · 38 comments
Open

Are decorators strict mode code? #204

nicolo-ribaudo opened this issue Jan 12, 2019 · 38 comments

Comments

@nicolo-ribaudo
Copy link
Member

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:

  • We can ignore this problem, and say that decorators will always be parsed as strict:
    with({}); // Ok
    @(() => { with({}); }) // Error
    function X() { with({}); } // Ok
  • We can parse decorators using the "strictness" of the surrounding context:
    @(() => { with({}); }) // Ok
    class X {
      @(() => { with({}); }) // Error
      foo() {}
    }
    "use strict";
    
    @(() => { with({}); }) // Error
    class X {
      @(() => { with({}); }) // Error
      foo() {}
    }
@ljharb
Copy link
Member

ljharb commented Jan 12, 2019

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
Copy link

@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
Copy link
Member

ljharb commented Jan 12, 2019

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
Copy link

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

@erights
Copy link

erights commented Jan 12, 2019

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member

@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
Copy link

erights commented Jan 13, 2019

Or to take option 3 of @ljharb 's list at #204 (comment) :

  • 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

This avoids lookahead, in addition to its other advantages #204 (comment)

@erights
Copy link

erights commented Jan 13, 2019

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
Copy link
Member

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
Copy link

erights commented Jan 13, 2019

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

@littledan
Copy link
Member

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

@loganfsmyth
Copy link

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
Copy link
Member

ljharb commented Jan 15, 2019

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

@nicolo-ribaudo
Copy link
Member Author

No, the current patch will also throw for @foo

@ljharb
Copy link
Member

ljharb commented Jan 15, 2019

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

@littledan
Copy link
Member

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.

@littledan littledan reopened this Jan 15, 2019
@nicolo-ribaudo
Copy link
Member Author

My preferred option from #204 (comment) is 1, followed by 2

@littledan
Copy link
Member

@erights expressed a concern about the first option in #204 (comment) , 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
Copy link
Member

Option 1 is drafted in #209.

@ljharb
Copy link
Member

ljharb commented Jan 15, 2019

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
Copy link
Member

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
Copy link
Member

ljharb commented Jan 15, 2019

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
Copy link
Member

@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
Copy link
Member

ljharb commented Jan 15, 2019

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

@littledan
Copy link
Member

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
Copy link
Contributor

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
Copy link

erights commented Jan 15, 2019

@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
Copy link

erights commented Jan 15, 2019

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';

@littledan littledan mentioned this issue Jan 15, 2019
6 tasks
@bakkot
Copy link

bakkot commented Jan 23, 2019

@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. tc39/ecma262#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
Copy link
Member

It looks like we came to a solid conclusion with #209 . We should pull this logic forward as the spec text for the new decorators proposal is drafted.

@pzuraq
Copy link
Collaborator

pzuraq commented Feb 2, 2024

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
Copy link

erights commented Feb 3, 2024

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

@ljharb
Copy link
Member

ljharb commented Feb 3, 2024

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
Copy link

erights commented Feb 3, 2024

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.

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

No branches or pull requests

8 participants